Skip to content

Add ADPimega as a submodule (branch: lumentum-v3)#95

Open
Gabriel-Amorim255 wants to merge 1 commit intoareaDetector:masterfrom
Gabriel-Amorim255:master
Open

Add ADPimega as a submodule (branch: lumentum-v3)#95
Gabriel-Amorim255 wants to merge 1 commit intoareaDetector:masterfrom
Gabriel-Amorim255:master

Conversation

@Gabriel-Amorim255
Copy link
Copy Markdown

Add ADPimega as submodule to interface with Pitec detectors.

@MarkRivers
Copy link
Copy Markdown
Member

I don't think this PR is ready.

  • This would be the only submodule of areaDetector that is not in the areaDetector project. That means that members of the areaDetector organization would not be able to modify it.
  • There is no docs/ directory. This is needed so that the areaDetector documentation includes documentation for all drivers.
  • The repository contains a copy of NDFile.template. https://github.com/cnpem/ADPimega/blob/master/pimegaApp/Db/NDFile.template. Why is this, and how has it been modified?
  • What is the purpose of the synApps folder? No other areaDetector repositories contain this.
  • The README.md contains a link to pimega-api under Dependencies. This link does not work.

@Gabriel-Amorim255
Copy link
Copy Markdown
Author

Hello Mark,

First, thank you for the review!

I had a feeling that a few things were missing, but I wanted to open this initial PR to gather feedback on how to properly prepare the final version.

I actually wanted to ask who I should contact—or what the process is—to have ADPimega added under the areaDetector project. I couldn’t find any clear instructions on how to do that, so any guidance would be greatly appreciated.

Regarding the documentation and the synApps folder: I’ve added the relevant submodules using the commit (07b881), which is the one currently in sync with the latest updates from the lumentum-v3 branch. However, the URL specified in the submodule points to master, which may be causing some confusion. Ideally, if we could add this lumentum-v3 branch under the areaDetector organization, that would be best, as we could continue to update it there. If using master is a requirement, we can reach out to CNPEM to request an update.

The README.md has also been updated in that branch, and now includes a clearer note on the dependency for the PIMEGA API:

This IOC requires the PIMEGA library to be installed in /usr/lib/ under libpimega.so. The include files must be added to /usr/local/include/pimega.

As for the copy of NDFile.template, I still need to investigate why it's present. I’ll look through the commit history to see if any modifications were made to it and run some tests to determine whether it's actually being used. If it turns out to be redundant, I’ll remove it.

Thanks again for your time and support!

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