Skip to content

base: add support for ADEiger module.#141

Merged
ericonr merged 1 commit intomainfrom
adeiger-support
Apr 8, 2026
Merged

base: add support for ADEiger module.#141
ericonr merged 1 commit intomainfrom
adeiger-support

Conversation

@gustavosr8
Copy link
Copy Markdown
Contributor

@gustavosr8 gustavosr8 commented Mar 4, 2026

Description

Add support for ADEiger module, in order to operate Pilatus4.

ZeroMQ library development package is required in the build stage of ADEiger module.

There are some unreleased features we would like to use, regarding Pilatus4 support, what makes necessary a checkout after initializing the submodule. The EDEIGER_VERSION points to the latest commit, but should point to R3-6 when the version were tagged.

Checklist

  • Follow the commit format specified in CONTRIBUTING.md;
  • Describe your user-facing changes in CHANGES.md, following the format
    established by previous entries.

@gustavosr8 gustavosr8 force-pushed the adeiger-support branch 2 times, most recently from a1d2306 to 9af49c9 Compare March 5, 2026 17:46
@gustavosr8 gustavosr8 marked this pull request as ready for review March 6, 2026 13:41
@gustavosr8 gustavosr8 requested review from ericonr, guirodrigueslima and henriquesimoes and removed request for guirodrigueslima March 6, 2026 13:42
Copy link
Copy Markdown
Member

@ericonr ericonr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commit message:

base: add support for ADEiger module.

base: add ADEiger module.

ZeroMQ library development package is required in the build stage of
ADEiger module.

This module uses libzmq to interact with the detector data stream.

(this way you justify the presence of the -dev package and the runtime package in your runtime image)

There are some unreleased features we would like to use, regarding
Pilatus4 support, what makes necessary a checkout after initializing the
submodule. The EDEIGER_VERSION points to the latest commit, but should
point to R3-6 when the version were tagged.

Since Pilatus4 support hasn't been released yet and we are interested in using
it, we are using the latest commit from the master branch instead of a tagged
release.

@gustavosr8 gustavosr8 requested a review from ericonr March 12, 2026 19:24
@gustavosr8 gustavosr8 marked this pull request as draft March 12, 2026 19:36
@gustavosr8 gustavosr8 marked this pull request as ready for review March 18, 2026 19:35
Copy link
Copy Markdown
Member

@ericonr ericonr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think tagging a new release is pending on further driver changes, right? So we have to use a commit for now.

@gustavosr8
Copy link
Copy Markdown
Contributor Author

I think tagging a new release is pending on further driver changes, right? So we have to use a commit for now.

Yeap, discussed here: areaDetector/ADEiger#70 (comment)

@gustavosr8 gustavosr8 requested a review from ericonr March 19, 2026 18:55
@ericonr
Copy link
Copy Markdown
Member

ericonr commented Mar 19, 2026

Well, there is something for us to choose here. Does this need to be merged before the tag? We're perfectly capable of building the IOC image ourselves, and use it for commissioning tests in the beamline. In the meantime, we can try and move along the missing functionality, so we can have everything merged before the machine stop ends.

@gustavosr8
Copy link
Copy Markdown
Contributor Author

Does this need to be merged before the tag?

Since we are talking about commissioning tests, there is no hurry on merging this, IMHO. (This is actually the reason for leaving this PR as draft on the first time)

But when comes the time to the official deploy for operation, if it hasn't been tagged yet, then we should use the untagged version.

@ericonr
Copy link
Copy Markdown
Member

ericonr commented Apr 6, 2026

#25 130.0 /usr/bin/g++ -o test_lz4  -L/opt/epics/modules/areaDetector/ADEiger/lib/linux-x86_64 -Wl,-rpath,/opt/epics/modules/areaDetector/ADEiger/lib/linux-x86_64          -rdynamic -m64         test_lz4.o    -ldectrisCompression  -llz4 
#25 130.0 /usr/bin/ld: cannot find -llz4: No such file or directory

is using lz4 wrongly. I will create a PR for this...

@gustavosr8
Copy link
Copy Markdown
Contributor Author

Just documenting, @ericonr's PR has been closed since Mark Rivers is addressing this differently in areaDetector/ADEiger#91. While it still unmerged, I think we should use the last stable commit, areaDetector/ADEiger@6ca17ce. What do you think?

@ericonr
Copy link
Copy Markdown
Member

ericonr commented Apr 8, 2026

As discussed, we need areaDetector/ADEiger@083243f for the TIFF fixes. We'll patch test_lz4 for now.

@ericonr
Copy link
Copy Markdown
Member

ericonr commented Apr 8, 2026

Please rebase properly onto main.

Copy link
Copy Markdown
Member

@ericonr ericonr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

I prefer [1] <link-content> directly, without the colon

@gustavosr8 gustavosr8 force-pushed the adeiger-support branch 2 times, most recently from ea9fab9 to 81a0592 Compare April 8, 2026 13:49
This module uses libzmq to interact with the detector data stream.

Since Pilatus4 support hasn't been released yet and we are interested in
using it, we are using the latest commit from the master branch instead
of a tagged release. Unfortunately, this commit also adds an unwanted
dependency on lz4 for the tests scripts, that are being removed in an
unmerged PR [1]. Add a patch to remove this dependency.

[1] areaDetector/ADEiger#91
@ericonr ericonr merged commit b2f1a38 into main Apr 8, 2026
3 checks passed
@ericonr ericonr deleted the adeiger-support branch April 8, 2026 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants