fix: Give better error message on non-existent device #1194
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1194 +/- ##
==========================================
- Coverage 94.54% 94.35% -0.19%
==========================================
Files 41 41
Lines 2566 2569 +3
==========================================
- Hits 2426 2424 -2
- Misses 140 145 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Having a different message for the device not existing and the device being the wrong type makes a lot of sense but I'm not sure about listing the available devices in the error. The list will include a whole load of devices that are also not valid while not including some that are, eg it would only list Possibly out of scope of this PR but would it also make sense to change the message for the invalid type case to include the type it actually is? At the moment it is only |
tpoliaw
changed the title
Fair enough but I do think that given this is the external API it's close to user facing so would like to be as helpful as possible. How about we list all known devices with the correct type and give the caveat that the list may not be exhaustive?
Yh, happy to do this |
Sure, that works. I think #1129 was the change for including all devices and would get rid of the caveat. |
| device_names = list(self.devices.keys()) | ||
| raise ValueError( | ||
| f"Device {value} cannot be found, " | ||
| f"available devices are: {device_names}" |
There was a problem hiding this comment.
Since devices are a tree and blueapi supports resolving them recursively (foo.bar.baz etc.), devices.keys() is not the complete set of available devices. So you might just get [sample_stage] instead of [sample_stage.x, sample_stage.y], for example. Not sure if it really matters here but worth knowing.
Relates to #1129
There was a problem hiding this comment.
Nevermind, @tpoliaw beat me to it
Fixes #1193