forked from JPHammonds/ADPICam
-
Notifications
You must be signed in to change notification settings - Fork 5
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
WIP: Multitrack and Improvements #15
Open
mfurseman
wants to merge
34
commits into
areaDetector:master
Choose a base branch
from
mfurseman:rbase
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Replace tab characters with spaces for consistent indentation.
In a few places, use the C99 __func__ construct instead of an explicit local variable.
Use subroutines, dammit! DRY.
Not needed as callbacks are used.
Make asynUser available for error reporting.
Duplicate picam library version (already in ADPICam-specific VersionNumber_SDK PV) into areaDetector standard SDKVersion.
Avoid compiler warnings from unused error status values and unhandled values in switch statements. Although the functioning of the software has not changed, the downside is that areas which need review (e.g. on how to handle such errors) may be more likely to be overlooked.
When we want to set a camera we would generally also want to set the values that are stored in that camera to the values which are stored in our PVs. This moves the logic so that the SDK is queried when one calls setSelectedCamera instead of having to do it separately.
If the user does not provide a string for the demo camera name then the null pointer is dereferenced resulting in undefined behavior. Instead it would be more helpful to print the valid list of demo cameras.
The PICam constructor would connect to a camera, either a physical camera if it is available or the Quadro4320 demo camera if not. It'd then call `setIntegerParam(PICAM_AvailableCameras` which results in the camera being uninitialized and then reinitialized again! I can't see why it was doing this, probably a leftover from some previous refactoring. I've rationalized this to connect to a camera at the end of the construction. A Demo camera is no longer default selected, the user can still add one from the iocsh.
mfurseman
changed the title
WIP: Multitrack and improvments
WIP: Multitrack and improvements
May 26, 2022
mfurseman
changed the title
WIP: Multitrack and improvements
WIP: Multitrack and Improvements
May 26, 2022
This was written to assume that one had to map from the PICam enumerations to the asyn enumerations when the readback value is sent. It turns out that asyn already does this (has that changed since this was written?), and attempting to translate the value twice resulted in the readbacks being randomly assigned!
…serial PICam appends ':Demo' to the serial anyway so users should still be aware when a demo and physical camera are both configured
This caused problems when two cameras had the same model name: a PV write using the string would always write to the first instance of the occurance
Dear Matthew, at ASDEX Upgrade we are also looking at including this module into our systems. Is this something you are still working on and we can contribute? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
At UKAEA we're rolling out several PI cameras for the MAST-U diagnostics; we've been doing quite a bit of work on this module. I think I (and colleagues) have tidied up the relevant changes enough to make them tenable to review commit-by-commit.
I've marked this as WIP as it depends on some tweaks to the mutliroi implementation in ADCore which we haven't created a PR for yet.