feat: add depth parameter to devices endpoints #912

DiamondJoseph · 2025-04-09T10:39:36Z

Fixes #903 by adding an optional parameter to the /devices endpoint

TODO: Add parameter to /devices/{device_name} endpoint
TODO: Better tests

DiamondJoseph · 2025-04-09T10:40:51Z

src/blueapi/service/interface.py

+        model
+        for device in context().devices.values()
+        for model in DeviceModel.from_device_tree(device, depth)
+        if model.protocols


Prevent any child devices that are not deserializable as have no protocols and so are not registered?

DiamondJoseph · 2025-04-09T10:41:14Z

src/blueapi/service/main.py

+            "-1 for all, 0 for just root",
+            ge=-1,
+            # https://github.com/fastapi/fastapi/discussions/13473
+            json_schema_extra={"description": None},


fastapi/fastapi#13473

I'll make an issue for tracking this if this is the way we want to go

DiamondJoseph · 2025-04-09T10:42:02Z

src/blueapi/service/model.py


 def _protocol_info(device: Device) -> Iterable[ProtocolInfo]:
    for protocol in BLUESKY_PROTOCOLS:
-        if isinstance(device, protocol):
+        if isinstance(device, protocol) and protocol is not AsyncDevice:


Prevent ProtocolInfo having "Device"

callumforrester · 2025-04-09T11:58:06Z

src/blueapi/service/main.py

+        int,
+        Query(
+            description="Maximum depth of children to return: "
+            "-1 for all, 0 for just root",


Could: Personally I'm not a fan of the "-1 means infinity" convention, would prefer this was explicit e.g. the type was int | Literial["infinite"] or even just int | None

I've just tried this out and neither click or fastapi are happy with it with their rich handling:

click cannot have a type as click.IntRange(min=0) | Literal["all"], as it tries to instantiate the union.
fastapi cannot have ge validation that applies to only the int side of the Union.

I would agree, but I don't think it's possible if we want to use the nice schemas.

callumforrester · 2025-04-11T07:48:47Z

src/blueapi/service/main.py

@@ -214,9 +216,21 @@ def get_plan_by_name(name: str, runner: WorkerDispatcher = Depends(_runner)):

 @secure_router.get("/devices", response_model=DeviceResponse)
 @start_as_current_span(TRACER)
-def get_devices(runner: WorkerDispatcher = Depends(_runner)):
+def get_devices(


Must: Add another attribute to DeviceModel showing where the device is in the tree, so you get back something like

{ "name": "x", "address": "stage.x" "protocols": [] }

The alternative is to return an actual tree structure and let the client work out for itself where the device is, but we need to provide that information somehow. I think a list of unique devices with tree address as an attribute is my preference.

If you do it that way then no, I don't think get_device should also return sub-devices. It just provides exactly that thing at the given address.

Why do we need to do that? For ophyd-async devices the name is parent_child and can be used to reference the child device when passing it in as params.

Two reasons:

The client can't actually reconstruct the tree from that information because of the differing python variable names from ophyd device names. For example your tree could be

stage (name="sample") - x (name="sample_x")

Blueapi will find x via a request for stage.x, not sample.x, so if I just have x in isolation I have no way to work that out.

Even if that wasn't the case though I don't like the idea of an API that technically does allow you to reconstruct the tree, but only by doing some string manipulation, it seems messy and not easy to protect with versioning. We should tell the user about the structure and tell them in a structured way.

See also a stackexchange thread that I read.

Why does the client need to reconstruct the tree? I need to know what devices can be used for what plans. If I need to know that these two motors are related, my plan should accept and I should be passing their mutual parent.

I'm probably not seeing the big picture here but if I have table: Table, table_x: Motor and table_y: Motor, I can write a generic plan(x: Motor, y: Motor) and a specific related_plan(table: Table): yield from plan(table.x, table.y)

Consider querying the server for its devices and then running a plan with said devices:

> blueapi controller devices table ... table_x ... table_y ... > blueapi run count '{"detectors": ["table_x"]}' Uh no, "table_x" not found!

The correct command is

-blueapi run count '{"detectors": ["table_x"]}' +blueapi run count '{"detectors": ["table.x"]}'

But you have no way of knowing that from the client output unless you just happen to know to convention or (even worse) code it into your client. The name mismatch is also still an issue here. Finally, is the device tree information that we want to hide?

Why do we preserve and parse ourselves the dot access of child devices, instead of registering the children of the device and allowing references to them by name? It means the call to blueapi uses different arguments than propagate downstream in documents: another name mismatch.

It's not that I want to hide the device tree, but it's outside of the scope of being able to run plans on child devices.

So you're saying don't accept table.x but do accept table_x? I guess the main problem is that when I'm running locally in my IPython terminal I type

[1]: table.children() ["x", "y"] [2]: RE(bp.count(detectors=[table.x]))

Knowing about/inspecting the tree is inherently part of that user experience so it is a bigger leap to completely hide it/expect the user to enter something different and think of the devices in a different way.

$ blueapi controller devices table: protocols: table_x: protocols: Movable: - float table_y: protocols: Movable: - float $ blueapi controller run count '{"detectors": ["table_x"]}'

Is it unreasonable to say "I think the user experience has already changed so much that swapping out . for _ doesn't really matter?" Or commit to supporting both. Either way, blueapi devices with some depth will be returning the Device.name, which blueapi considers special and unique, which uses _

No idea, I would suggest we ask some users

callumforrester · 2025-04-11T07:50:25Z

src/blueapi/service/model.py

@@ -40,10 +42,35 @@ def from_device(cls, device: Device) -> "DeviceModel":
        name = device.name if isinstance(device, HasName) else _UNKNOWN_NAME
        return cls(name=name, protocols=list(_protocol_info(device)))

+    @classmethod
+    def from_device_tree(cls, root: Device, max_depth: int) -> list["DeviceModel"]:


Should: There is a function in device_lookup.py that also traverses the device tree, could extract the walk logic into a generator and use it in both of these.

codecov · 2025-04-14T12:39:13Z

Codecov Report

Attention: Patch coverage is 82.05128% with 7 lines in your changes missing coverage. Please review.

Project coverage is 94.08%. Comparing base (39c7371) to head (f8a9f27).

Files with missing lines	Patch %	Lines
src/blueapi/service/model.py	70.83%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #912      +/-   ##
==========================================
- Coverage   94.32%   94.08%   -0.24%     
==========================================
  Files          39       39              
  Lines        2360     2385      +25     
==========================================
+ Hits         2226     2244      +18     
- Misses        134      141       +7

☔ 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.

DiamondJoseph commented Apr 9, 2025

View reviewed changes

DiamondJoseph changed the title ~~Add depth parameter~~ Add depth parameter to devices endpoints Apr 9, 2025

callumforrester requested changes Apr 11, 2025

View reviewed changes

DiamondJoseph force-pushed the device-depth branch from 6df4fdf to f8a9f27 Compare April 14, 2025 12:37

Add depth parameter to get_devices

b1d5439

DiamondJoseph force-pushed the device-depth branch from f8a9f27 to b1d5439 Compare April 16, 2025 16:05

DiamondJoseph changed the title ~~Add depth parameter to devices endpoints~~ feat: add depth parameter to devices endpoints Apr 16, 2025

feat: add depth parameter to devices endpoints #912

Are you sure you want to change the base?

feat: add depth parameter to devices endpoints #912

Uh oh!

Conversation

DiamondJoseph commented Apr 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DiamondJoseph Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Apr 14, 2025

Codecov Report

Uh oh!

Uh oh!

DiamondJoseph commented Apr 9, 2025 •

edited

Loading

DiamondJoseph Apr 14, 2025 •

edited

Loading