Skip to content
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

Improve DeviceInitializationController Mock Parameter Handling in dodal #964

Open
ZohebShaikh opened this issue Dec 16, 2024 · 3 comments
Open

Comments

@ZohebShaikh
Copy link
Contributor

A brief description of the issue:
The mock parameter in the DeviceInitializationController API is only meaningful when connect_immediately=True. Otherwise, it is ignored, and users need to inspect the factory code to determine its behavior. This makes the API confusing and difficult to use, especially in cases requiring clarity on mock handling. There is a related issue in blueapi to be worked on after this

This issue is not beamline-specific but arises from general usage challenges with the API. Addressing it will improve usability and make the API more intuitive for all developers.
Acceptance Criteria

  • The behavior of the mock parameter is clearly documented, including its dependency on connect_immediately.
  • Any misleading or unused functionality related to the mock parameter is addressed or renamed for clarity (e.g., mock_if_connect_immediately).
  • Developers can easily understand how to configure mock without needing to inspect the factory code.
@callumforrester
Copy link
Contributor

Considering the different permutations:

connect_immediately=False connect_immediately=True
mock=False No effect Connect to real hardware
mock=True Currently no effect, but we'd like it to somehow "mark" the device when we do connect it Connect to mock hardware

Would an enum make more sense for this? e.g.

@device_factory(connect=ConnectionStrategy.Eager)  # Connect immediately, do not mock
def device_a() -> DeviceA: ...


@device_factory(connect=ConnectionStrategy.EagerMock) # Connect immediately, mock
def device_b() -> DeviceB: ...


@device_factory(connect=ConnectionStrategy.LazyMock) # Don't connect, expect blueapi et al. to read this property and act accordingly
def device_c() -> DeviceC: ...

Another thing we could do is add a connect(device) method to DeviceInitializationController which follows the agreed strategy.

Thoughts welcome

@DiamondJoseph
Copy link
Contributor

Regardless of whether the device_factory method changes, the value is still cached on the device factory and requires that blueapi (etc.) has access to it, not just the device- meaning that dodal can't define a "get_all_devices" that just returns unconnected devices, as other services need to know if a device should be connected to a mock backend.

@callumforrester
Copy link
Contributor

Yes, the API should allow us to explicitly define "can be mock" vs "should always be mock"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants