Skip to content

Make BlueskyContext accept all objects with a name property as devices#147

Merged
callumforrester merged 1 commit intomainfrom
accept-named-devices
Apr 24, 2023
Merged

Make BlueskyContext accept all objects with a name property as devices#147
callumforrester merged 1 commit intomainfrom
accept-named-devices

Conversation

@callumforrester
Copy link
Copy Markdown
Contributor

Previously, BlueskyContext only accepted objects that conformed to the Readable or Flyable protocols as devices. This is inherited from the days when all devices conformed to these protocols or their inheritors. Now the protocols no longer have an inheritance heirachy and are mixed and matched through multiple inheritance. The only sensible criteria is whether an object has a name property. This opens the door to abuse, storing objects that are definitely not devices, but that may be the price to pay for the required flexibility.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2023

Codecov Report

Merging #147 (5e7f66d) into main (90b4710) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #147   +/-   ##
=======================================
  Coverage   74.52%   74.52%           
=======================================
  Files          37       37           
  Lines        1048     1048           
=======================================
  Hits          781      781           
  Misses        267      267           
Impacted Files Coverage Δ
src/blueapi/core/context.py 76.66% <100.00%> (-0.39%) ⬇️
src/blueapi/utils/base_model.py 100.00% <100.00%> (ø)
src/blueapi/utils/serialization.py 87.50% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Copy Markdown
Contributor

@DiamondJoseph DiamondJoseph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later consideration: should device no longer allow explicitly setting an override name, device.name is the only name it can be registered with, and all devices must be HasName?

@callumforrester
Copy link
Copy Markdown
Contributor Author

Agreed, made an issue: #148

@callumforrester callumforrester merged commit e27e864 into main Apr 24, 2023
@callumforrester callumforrester deleted the accept-named-devices branch April 24, 2023 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants