P11 develop

[agruzinov]

Here is another round of debug after upgrading to the latest code (mostly in DESY HO).

[beteva]

Dear @agruzinov, there is a way to add extra values, without redefining the BaseValueEnum. The values are always accessible via self.VALUES, as you can find it attributed in this case in AbstractShutter. If you want to add extra values to this enum, you can add it like this:

def _initialise_values(self):
        """Add additional, known in advance states to VALUES"""
        values_dict = {item.name: item.value for item in self.VALUES}
        values_dict.update(
            {
                "MOVING": "In Motion",
                "UNUSABLE": "Temporarily not controlled",
            }
        )
        self.VALUES = Enum("ValueEnum", values_dict)

And in the init, you simply add: self._initialise_values() Thus you simply add whatever you need. Please have a look in the ShutterMockup

[rhfogh]

What is the relationship between AbstractNState, AbstractMotor, and MotorsNPosition? WOuld it be possible if MotorsNPosition inherited from one of the former?

[vrey01]

Dear Rasmus,

An object could be "not open" while still not being closed. With this shutter, that is the case, the open and close states are read from hardware from different signals.

El mar, 17 oct 2023 a las 11:35, Rasmus H. Fogh @.***>) escribió:

@.**** commented on this pull request.

In mxcubecore/HardwareObjects/DESY/P11DetectorCover.py https://github.com/mxcube/mxcubecore/pull/800#discussion_r1361816408:

• @property def is_closed(self):

Can't we get rid of is_closed altogether? If you just write 'not is_open' everywhere that would mean one function less - and arguably a duplication less.

— Reply to this email directly, view it on GitHub https://github.com/mxcube/mxcubecore/pull/800#pullrequestreview-1681844530, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKDQTYJ27BAHHOQBLAS7F3X7ZGOVAVCNFSM6AAAAAA5UJV3ZSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTMOBRHA2DINJTGA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

--

Bixente

[vrey01]

Rasmus,

thanks for your comments.

In this case, MotorsNPosition is a specific, still general object, that allows to define a state for a number of motors together, not only for one of them.

El mar, 17 oct 2023 a las 11:46, Rasmus H. Fogh @.***>) escribió:

What is the relationship between AbstractNState, AbstractMotor, and MotorsNPosition? WOuld it be possible if MotorsNPosition inherited from one of the former?

— Reply to this email directly, view it on GitHub https://github.com/mxcube/mxcubecore/pull/800#issuecomment-1766061505, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKDQT6FR6BZNU54ZDFIUULX7ZHZDAVCNFSM6AAAAAA5UJV3ZSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONRWGA3DCNJQGU . You are receiving this because you are subscribed to this thread.Message ID: @.***>

--

Bixente

[rhfogh]

@vrey01 All makes sense.

I kind of wonder what state a shutter is in if it is neither open nor closed. But then, I am never going to interact with it directly, so it is fine with me.

[marcus-oscarsson]

Nicely done, I found a few things I found interesting (see my comments/questions above).

Apart from that as @beteva and @rhfogh, pointed out there is perhaps some duplication that you can get rid off.

Also as @fabcor-maxiv mentioned be careful with the get_properties method. You can perhaps make it a bit more specific and call it get_motor_properties ?

[fabcor-maxiv]

@agruzinov Something seems wrong with this pull request. Maybe some rebase gone wrong?

[agruzinov]

Hmm, ok, I will check it. Meanwhile maybe it is better to put is as a WIP.

[fabcor-maxiv]

Meanwhile maybe it is better to put is as a WIP.

I converted the pull request to "draft"

[beteva]

Hi @agruzinov, There are a lot of changed file, which I believe are not relevant to P11. Have you done git rebase before you made the PR?

[agruzinov]

@beteva: Yes, this is exactly what I've done...

[agruzinov]

@fabcor-maxiv

_get_property vs. get_property, see earlier inline comment

Sorry, I've did not get exactly.

[agruzinov]

Thanks everyone for the comments! I'm still on the shutter values (thanks @beteva ). I hope it can soon be merged so the next PRs will be smaller.

Cheers!

[marcus-oscarsson]

Iam sorry @agruzinov I lost track of things, is this PR still active ?

[agruzinov]

In principle yes. I guess the main issue for now is the some problems with rebase. Maybe it is better to close it for now and open a new PR then??

[agruzinov]

PR needs to be resubmitted. Closing.