Use config server for detector LUT

[jacob720]

Fixes DiamondLightSource/mx-bluesky#1509

Required by DiamondLightSource/mx-bluesky#1510

Instructions to reviewer on how to test:

1. Check detector config is being read through the config server rather than directly from the filesystem

Checks for reviewer

• Would the PR title make sense to a scientist on a set of release notes
• If a new device has been added does it follow the standards
• If changing the API for a pre-existing device, ensure that any beamlines using this device have updated their Bluesky plans accordingly
• Have the connection tests for the relevant beamline(s) been run via dodal connect ${BEAMLINE}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.11%. Comparing base (14b2a03) to head (072bf2f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1776   +/-   ##
=======================================
  Coverage   99.11%   99.11%           
=======================================
  Files         318      318           
  Lines       12331    12336    +5     
=======================================
+ Hits        12222    12227    +5     
  Misses        109      109           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[oliwenmandiamond]

Is it not simpler to just pass in the ConfigClient instance to the constructor?

I dislike this because some additional hidden env configuration someone has to set up and we won't know it fails until you run it. If you don't pass in the ConfigClient to the constructor, type checking would complain instantly

[jacob720]

I basically agree, but the issue with passing it in is that DetectorParams then needs to hold an instance of the config client, which seems a bit strange for a parameter model.

@cached_property
    def beam_xy_converter(self) -> DetectorDistanceToBeamXYConverter:
        return DetectorDistanceToBeamXYConverter(self.det_dist_to_beam_converter_path)

Would need to become

@cached_property
    def beam_xy_converter(self) -> DetectorDistanceToBeamXYConverter:
        return DetectorDistanceToBeamXYConverter(
            self.det_dist_to_beam_converter_path, self.config_client
        )

If you still think that's nicer I can do that though. It also might cause issues when trying to serialise the parameter model but I can't see this is ever done so that's probably fine.

[oliwenmandiamond]

This is fine. This way, we have much more control via dependency injection. This doesn't need to be a cached_property either. You don't want the state to persist between tests, it needs to be reset for each one.

[jacob720]

I'll do that then. As for it being cached, the params should be reconstructed for each test regardless on whether it's cached. Caching means in actual plans we don't rebuild the lut every time we want to read it

[rtuck99]

I think that if a fixture is not required by any device factory, it will not be called, so set_config_client() will not be invoked.

So you probably need to include it as an argument to the device factory even if the factory doesn't use it.

[jacob720]

Have removed the fixture and just call set_config_client now

[rtuck99]

Approved