-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add adsDriver and autoparamDriver #69
Add adsDriver and autoparamDriver #69
Conversation
Note that this is a different library from #45 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing, thanks a lot!
To answer your question about varname
, it has to do with how EPICS resolves dependencies. For example, in the case adsDriver
, to tell EPICS where autoparamDriver
is, you put a line in configure/RELEASE
:
AUTOPARAM=/path/to/autoparamdriver
varname
is the name of the variable. The only technical constraint I know is that it must be unique per package.
Changes looks good, I only have minor comments. Could you split your changes into 2 commits, and run nix fmt
? I also like to keep varname
near pname
and version
.
Thanks again!
Oh, another question: do you think it would be possible to have an automated test for these packages? |
And to do that if you want to dive in, @minijackson find this python lib if you need : https://pyads.readthedocs.io/en/latest/documentation/testserver.html And example of ioc tests are here : https://github.com/epics-extensions/EPNix/tree/master/ioc/tests/support I know that because I am supposed to do so for the other ADS driver :) |
History should be clean now
Changed this as well
While I really like the NixOS test driver (did quite a bit of work on it back in the day), I can't really justify this to my employer atm, so don't expect this in the short run. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks!
Hmm the driver uses sum reads, and the testServer doesn't seem to support this, so this requires upstream additions to the test server. (EDIT: talking about the NixOS test for this component) |
Add adsDriver (https://epics.cosylab.com/documentation/adsDriver/) and autoParamDriver (dependency, https://epics.cosylab.com/documentation/autoparamDriver)
I'm not sure what is meant with the mandatory varname attribute (fairly new to EPICS) so I'm not sure if I got those correct, would like review on that.
Added myself as maintainer for these two.