Updated versions of DESY P11 HO (PR 800, 854) #856
Conversation
|
Hi @agruzinov, all looks fine for me. Thanks for the effort. |
| sys.path.append("/opt/dectris/albula/4.0/python/") | ||
|
|
||
| from mxcubecore.BaseHardwareObjects import HardwareObject | ||
|
|
There was a problem hiding this comment.
I like the idea of this the AlbulaView, as I understand it it starts Albula on the same machine as MXCuBE is running and gets the images from the detector zero-mq stream ?
There was a problem hiding this comment.
Yes, but it is not completely implemented. Also in the python 3.11 this line leads to the segfault anyway. This will require a bit more work yet.
There was a problem hiding this comment.
Ok, i see. Did you also foresee to display a certain image on demand ?
There was a problem hiding this comment.
Ideally it is working right now with in-house software as a standard viewer (made by Jan). It shows the monitor interfaces during data collection and than changes to the recently collected file and shows it afterwards, so one can scroll back if needed. I was trying to understand whether the Adxv can show anything during data collection, but before hdf5 is saved, there is no way I guess.
There was a problem hiding this comment.
Hm, not sure. I don't think so
| # self.omega_mv(init_pos, self.default_speed) | ||
| self.collect_std_collection(start_at, stop_angle) | ||
|
|
||
| # This part goes to standard collection. Otherwise it produces phantom openings. |
There was a problem hiding this comment.
A general good practice is to remove commented code
| @@ -236,15 +267,24 @@ def input_from_params(self, data_collection, char_params): | |||
| acquisition_parameters = data_collection.acquisitions[0].acquisition_parameters | |||
| path_template = data_collection.acquisitions[0].path_template | |||
|
|
|||
| # Make sure there is a proper path conversion between different mount points | |||
| print( | |||
There was a problem hiding this comment.
Consider using log instead of print ?
|
Looks good to me, just a few comments. |
| @@ -340,11 +340,11 @@ def collect_characterisation( | |||
| self.omega_mv(start_angle, self.default_speed) | |||
|
|
|||
| for img_no in range(nimages): | |||
| print("collecting image %s" % img_no) | |||
| logging.info("collecting image %s" % img_no) | |||
There was a problem hiding this comment.
I guess here in this case it would be better to use self.log instead of logging . And maybe also debug instead of info. So maybe self.log.debug.
[Side note: Logging is all over the place in mxcube, we should seriously look at it at some point. Maybe set up a clear policy, and if possible enforce it with some linting. Feels like everyone writes whatever they feel like on the moment, no one looks at the existing code base or at the docs.]
There was a problem hiding this comment.
It would indeed be very nice to cleanup the logging, then I think we should be bit more liberal for site specific files like this one.
There was a problem hiding this comment.
Thanks for the comment. I still need all these logs/debugs because the system is still in active change. Once it is stabilised, the excessive logs should indeed go. But for now, it is hard to debug things without them.
|
The linting step is failing, Ill merge when its fixed :). Thanks ! |
|
black and autopep8 are ran on the code but it is not yet happy still... What could it be? |
Autoflake: https://github.com/mxcube/mxcubecore/actions/runs/8063131751/job/22024333705?pr=856#step:6:29 You should be able to run |
|
@agruzinov do the follwing in your shell |
|
Great ! 👍 |
As discussed in PR 800/854 here is the DESY P11 HO objects cumulative PR.