Shared devices discussion outcome

@oliwenmandiamond

Meeting attended by myself, @jacob720, @oliwenmandiamond, @RJCD-Diamond, @EmsArnold, @fajinyuan, @Relm-Arrowny

We came up with some decisions relating to #1490, #1554, #1532, #503

Q: What should my device factory files (eg i04.py) look like for beamlines which share devices
A: For devices which have the same config, these factories can live in a shared file (eg i04_shared.py). Need to check with @tpoliaw that this is compatible with the new device manager.

Q: How should dodal ensure that shared devices can only be written to by the permitted beamline?
A: GDA uses a user office database to check which beamline+visit has permission to use devices at which times. This database is just mapping visits to beamtime and beamline. See here for a bit more detail. We should do this check in dodal. For this, we require:

Working authn/authz
A decorator for the writable shared devices so that requests to write will check if the user has beamline. Some users (eg beamline scientists) could have permission to always write to the files.
When BlueAPI sends info to numtracker about the user/visit, these services can also provide dodal with the information about if the beamline has access to these devices and for how long. Will need discussion with core for best implementation details.

We didn't discuss who or when this should be done, but it's probably the best long term solution

Q: Can we move our device configuration to be in yaml files outside of dodal?
A: We all thought this would be good. @fajinyuan's team are happy to put some effort into this in the new year, and will involve other teams for design opinions.

Acceptance Criteria

We document these decisions in the dodal docs

@tpoliaw

A: For devices which have the same config, these factories can live in a shared file (eg i04_shared.py). Need to check with @tpoliaw that this is compatible with the new device manager.

In the current form you can use two device managers in blueapi and both sets of devices will be loaded:

sources:
  - kind: deviceManager
    module: dodal.beamlines.i04
  - kind: deviceManager
    module: dodal.beamlines.i04-shared

What is currently not supported is using devices from one as arguments/fixtures for another, eg if you have def stagexy(x, y): ... where x and/or y is defined in a different device manager.

If this was required, it would be possible to add an inherit/include/extend (bikeshed as appropriate) method to include all the devices of a different manager and make them available to the the current one. In this case, only the combined manager should be included in blueapi or some devices would be duplicated.

This is all from the perspective of blueapi. If multiple modules are required at once in dodal, I imagine some form of the second option would work as well, eg dodal connect i04 in the CLI would connect the inherited devices as well.

For the CLI use in the current version, you could have something like from i04-shared import devices as shared_devices and then use dodal connect i04 -n shared_devices to override which device manager it's using.

GDA uses a user office database to check which beamline+visit has permission to use devices at which times

Where is this done in GDA?

@fajinyuan

GDA uses a user office database to check which beamline+visit has permission to use devices at which times

Where is this done in GDA?

Good question, @fajinyuan do you know which parts of the GDA code does this?

The Java class is at gda.aop.CheckAccessBeforeMethod which is used in Spring bean definition using Spring AOP support.

<!-- do access check method intercepter -->
<bean id="accessCheckAdvice" class="gda.aop.CheckAccessBeforeMethod">
	<property name="accessControl" ref="accessControl" />
</bean>
<bean id="accessCheckAdvisor" class="org.springframework.aop.support.NameMatchMethodPointcutAdvisor">
	<property name="mappedNames" value="asynchronousMoveTo, rawAsynchronousMoveTo" />
	<property name="advice" ref="accessCheckAdvice" />
</bean>	
<!-- Plane Grating Monochromator CFF -->
<bean id="cff" class="org.springframework.aop.framework.ProxyFactoryBean">
	<property name="target">
		<bean class="gda.device.controlpoint.EpicsControlPoint">
			<property name="name" value="cff"/>
			<property name="pvNameGetPoint" value="BL06I-OP-PGM-01:CFF" />
			<property name="pvNameSetPoint" value="BL06I-OP-PGM-01:CFF" />
			<property name="upperLimit" value="20.0"/>
			<property name="lowerLimit" value="1.5"/>
		</bean>
	</property>
	<property name="proxyTargetClass" value="true" />
	<property name="interceptorNames">
		<list>
			<value>accessCheckAdvisor</value>
		</list>
	</property>
</bean>
<!-- Plane Grating Monochromator grating offset -->
<bean id="pgmgo" class="org.springframework.aop.framework.ProxyFactoryBean">
	<property name="target">
		<bean class="gda.device.controlpoint.EpicsControlPoint">
			<property name="name" value="pgmgo"/>
			<property name="pvName" value="BL06I-OP-PGM-01:GRTOFFSET" />
		</bean>
	</property>
	<property name="proxyTargetClass" value="true" />
	<property name="interceptorNames">
		<list>
			<value>accessCheckAdvisor</value>
		</list>
	</property>
</bean>

<!-- Plane Grating Monochromator mirror offset -->
<bean id="pgmmo" class="org.springframework.aop.framework.ProxyFactoryBean">
	<property name="target">
		<bean class="gda.device.controlpoint.EpicsControlPoint">
			<property name="name" value="pgmmo"/>
			<property name="pvName" value="BL06I-OP-PGM-01:MIROFFSET" />
		</bean>
	</property>
	<property name="proxyTargetClass" value="true" />
	<property name="interceptorNames">
		<list>
			<value>accessCheckAdvisor</value>
		</list>
	</property>
</bean>

In the current form you can use two device managers in blueapi and both sets of devices will be loaded:

sources:

kind: deviceManager
module: dodal.beamlines.i04

kind: deviceManager
module: dodal.beamlines.i04-shared

What is currently not supported is using devices from one as arguments/fixtures for another, eg if you have def stagexy(x, y): ... where x and/or y is defined in a different device manager.

If this was required, it would be possible to add an inherit/include/extend (bikeshed as appropriate) method to include all the devices of a different manager and make them available to the the current one. In this case, only the combined manager should be included in blueapi or some devices would be duplicated.

This is all from the perspective of blueapi. If multiple modules are required at once in dodal, I imagine some form of the second option would work as well, eg dodal connect i04 in the CLI would connect the inherited devices as well.

For the CLI use in the current version, you could have something like from i04-shared import devices as shared_devices and then use dodal connect i04 -n shared_devices to override which device manager it's using.

Just a thought, but if we have shared devices, could we not use a single device manager?

e.g

beamline iXX-shared.py

devices = DeviceManager()

@devices.factory()
def shared_device() -> Device:
     ...

Then in my main end station beamline, I do this

iXX.py

from dodal.beamlines.iXX_shared import devices

@devices.factory()
def beamline_device() -> Device:
    ...

Now I believe when doing dodal connect on beamline iXX, it should include all devices from main beamline and shared as using the same instance of DeviceManager. When doing dodal connect beamline iXX-shared, it will only include shared.

When using with blueapi and including iXX, it should pull in shared devices too? I haven't tested, but I think this should all work in theory with how it is implemented currently and doesn't overcomplicate it?

I would also add that if the above does work as expected, wouldn't it make sense to make DeviceManager a singleton?

I have tested this with dodal connect, and it does indeed work!

$ dodal connect test_shared -s
Attempting connection to test_shared (using dodal.beamlines.test_shared)
2 devices connected (sim mode):
        motor_shared_a
        motor_shared_b

$ dodal connect test_a -s
Attempting connection to test_a (using dodal.beamlines.test_a)
4 devices connected (sim mode):
        motor_shared_a
        motor_shared_b
        motor_test_a
        motor_test_b

The only issue I can think of is when shared is for multiple beamlines. So lets take i09 as an example

It has i09, i09-1 and i09-2, with shared components coming from i09-1-shared (shared between i09 and i09-1) and i09-2-shared (shared between i09 and i09-2).

In this scenario, the above wouldn't work as i09 would need to pull in the device manager from i09-1-shared and i09-2-shared which would be separate instances and wouldn't know which one to use.

I think the only way to resolve this would be to be able to combine the device managers (so singleton idea doesn't work...)? Maybe something like this

i09.py

from dodal.beamlines.i09_1_shared import devices as i09_1_shared_devices
from dodal.beamlines.i09_2_shared import devices as i09_2_shared_devices

devices = DeviceManager(i09_1_shared_devices, i09_2_shared_devices)

@devices.factory()
def motor() -> Motor:
   ...

It could work to import and re-use a device manager but it means importing one module can affect how a different module behaves which could be surprising, eg if both i06 and i06-1 both use i06-shared, importing i06 would mean building i06-1's devices would include devices from i06 as well.

I think if you import i06 and it imports i06-1 devices, then you would have it configured wrong?

i06-shared defines the DeviceManager

Correct way
i06 imports i06-shared devices
i06 is pulled in via dodal connect / blueapi
includes i06 and i06-shared devices

Correct way
i06-1 imports i06-shared devices
i06-1 is pulled in via dodal connect / blueapi
includes i06-1 and i06-shared devices

The way you would do it wrong is via this

i06-1 imports i06 devices
i06-1 is pulled in via dodal connect / blueapi
includes i06, i06-1 and i06-shared devices - wrong, used the wrong endpoint to link the devices together

I think this is easily solved by documentation and explaining how the DeviceManager works to link together beamlines? Unless I'm missing something else?

Sure, you could make it so that it works on the happy path but having a way for it to break if you hold it wrong seems fragile even if you write docs to say don't do it.

I personally don't think that holds up though because all beamlines are fragile and can break if not configured correctly by importing the wrong thing or miss typing something... That is why we have documentation, examples and tests...

I think this is the simplest most logical way to link together shared components of beamlines as the existing infrastructure is already there as I've proven it can work and just needs a small tweak for multiple shared beamlines

Fair enough, if it works and keeps the configuration simple, go for it.

I can create a PR that does an example shared beamline with documentation and maybe some added tests and see what we think

I've had a play around with this here #1741. So from my testing, we in theory are able to import directly the device manager from shared but we run into complications with pytest. The tests pass on the PR, but locally you will get warnings from pytest

  File "/workspaces/dodal/src/dodal/beamlines/i05_1.py", line 13, in <module>
    @devices.factory()
     ^^^^^^^^^^^^^^^^^
  File "/workspaces/dodal/src/dodal/device_manager.py", line 430, in decorator
    raise ValueError(f"Duplicate factory name: {func.__name__}")
ValueError: Duplicate factory name: sm
=========================== short test summary info ============================
ERROR src/dodal/beamlines/i05_1.py - ValueError: Duplicate factory name: sm
!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!
==================== 1699 tests collected, 1 error in 5.62s ====================

2025-11-28 13:03:45.779 [error] Subprocess exited unsuccessfully with exit code 2 and signal null on workspace /workspaces/dodal
2025-11-28 13:03:45.779 [error] pytest test discovery error for workspace:  /workspaces/dodal 
  
 The python test process was terminated before it could exit on its own, the process errored with: Code: 2, Signal: null for workspace /workspaces/dodal

This is because pytest will pull all the modules used for testing into one, meaning i05 and i05-1 are put in the same environment and not isolated from each other so then one of them fails as we have "duplicate" device as they both define "sm" on their respective beamline .

After experimenting more, I have managed to overcome this if we were to do something to effect of below instead

from dodal.beamlines.i05_shared import devices as i05_shared_devices
...
devices = DeviceManager()
devices.combine(i05_shared_devices)

We combine the devices from i05_shared into this beamline so they all found. The above successfully allows dodal connect to pull together the shared devices and beamline devices, tests pass and and no warnings from pytest inside VSCode

Navigation Menu

Repository navigation

Shared devices discussion outcome #1739

Acceptance Criteria

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Shared devices discussion outcome #1739

Description

Acceptance Criteria

Activity

tpoliaw commented on Nov 27, 2025

tpoliaw commented on Nov 27, 2025

olliesilvester commented on Nov 27, 2025

fajinyuan commented on Nov 27, 2025

oliwenmandiamond commented on Nov 27, 2025

oliwenmandiamond commented on Nov 28, 2025

tpoliaw commented on Nov 28, 2025

oliwenmandiamond commented on Nov 28, 2025

tpoliaw commented on Nov 28, 2025

oliwenmandiamond commented on Nov 28, 2025

tpoliaw commented on Nov 28, 2025

oliwenmandiamond commented on Nov 28, 2025

oliwenmandiamond commented on Nov 28, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions