mxcube / mxcubecore

Backend used by MXCuBE
http://mxcube.github.io/mxcube/
GNU Lesser General Public License v3.0
11 stars 52 forks source link

Add test for other than open/close values. #774

Closed beteva closed 1 year ago

beteva commented 1 year ago

Implemented more values in the ShutetrMockup to allow different than OPEN/CLOSED values to be set and tested. Reversed a line in AbstractShutter, removed by inadvertence.

github-actions[bot] commented 1 year ago

Coverage

Coverage Report •
FileStmtsMissCoverMissing
TOTAL58653543617% 
report-only-changed-files is enabled. No files were changed during this commit :)

Tests Skipped Failures Errors Time
1921 0 :zzz: 0 :x: 0 :fire: 1m 53s :stopwatch:
beteva commented 1 year ago

This may be just me misunderstanding, but near as I can see, AbstractShutter.BaseValueEnum is not imported or used anywhere, Which would mean that the BaseValueEnum used in ShutterMockup comes from AbstractNState, and contains only the value UNKNOWN. Which does not look right.

In AbstractShutter, there is a new BasaeValueEnum defined, which has OPEN and CLOSE members on top of the UNKNOWN. This is what makes it shutter, rather than simply NState - you guarantee OPEN and CLOSE. The ShutterMockup can be used as a model to implement other values, if needed, e.g. at ESRF we need MOVING and EXTERNAL.

rhfogh commented 1 year ago

Yeah, I did notice that, but AFAICS the new BaseValueEnum is not used anywhere. It is defined in AbstractShutter.py, but it is not used inside that file, and it is not imported anywhere else. All imports are of AbstractNState.BaseValueEnum - and since BaseValueEnum is not inside the class but only in the file, I cannot see how it could be inherited anywhere either. When ShutterMockup references self.VALUES, near as I can see it inherits the VALUES class attribute that is set in AbstractNState, where BaseValueEnum is defined as containing only "UNKNOWN".

Would you not need a line saying VALUES = BaseValueEnum in AbstractShutter?

beteva commented 1 year ago

Would you not need a line saying VALUES = BaseValueEnum in AbstractShutter?

yes, there is such a line, right after the declaration of the class

rhfogh commented 1 year ago

Me bad. I was looking in the code (locally) of the develop branch, not of the shutter_test branch.

My apologies.

beteva commented 1 year ago

Me bad. I was looking in the code (locally) of the develop branch, not of the shutter_test branch.

My apologies.

No problem. With the previous PR this line has been removed by inadvertence. I've put it back.