Allow setting on hardware backed signals#1078
Conversation
src/dodal/common/signal_utils.py
Outdated
| def __init__( | ||
| self, | ||
| get_from_hardware_func: Callable[[], Coroutine[Any, Any, SignalDatatypeT]], | ||
| set_to_hardware_func: Callable[[], Coroutine[SignalDatatypeT, Any, Any]] = None, |
There was a problem hiding this comment.
I think set_to_hardware_func should take a set value?
| set_to_hardware_func: Callable[[], Coroutine[SignalDatatypeT, Any, Any]] = None, | |
| set_to_hardware_func: Callable[[SignalDatatypeT], Coroutine[SignalDatatypeT, Any, Any]] | |
| | None = None, |
There was a problem hiding this comment.
Thanks! I missed it 'cos the linter is still broken due to #1072
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1078 +/- ##
=======================================
Coverage 97.74% 97.75%
=======================================
Files 170 170
Lines 6841 6849 +8
=======================================
+ Hits 6687 6695 +8
Misses 154 154 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Relm-Arrowny
left a comment
Relm-Arrowny
left a comment
There was a problem hiding this comment.
As I try to use it, I spotted a couple of things.
| def create_rw_hardware_backed_soft_signal( | ||
| datatype: type[SignalDatatypeT], | ||
| get_from_hardware_func: Callable[[], Coroutine[Any, Any, SignalDatatypeT]], | ||
| set_to_hardware_func: SetHardwareType, | ||
| units: str | None = None, | ||
| precision: int | None = None, | ||
| ): |
There was a problem hiding this comment.
never mind, I notice, it will always get overwrite.
| def create_rw_hardware_backed_soft_signal( | |
| datatype: type[SignalDatatypeT], | |
| get_from_hardware_func: Callable[[], Coroutine[Any, Any, SignalDatatypeT]], | |
| set_to_hardware_func: SetHardwareType, | |
| units: str | None = None, | |
| precision: int | None = None, | |
| ): | |
| def create_rw_hardware_backed_soft_signal( | |
| datatype: type[SignalDatatypeT], | |
| get_from_hardware_func: Callable[[], Coroutine[Any, Any, SignalDatatypeT]], | |
| set_to_hardware_func: Callable[[SignalDatatypeT], Coroutine[Any, Any, None]], | |
| initial_value: SignalDatatypeT | None = None, | |
| units: str | None = None, | |
| precision: int | None = None, | |
| ) -> SignalRW: |
There was a problem hiding this comment.
Yh, I think it's an edge case anyway and initial_value is probably never going to be None
src/dodal/common/signal_utils.py
Outdated
| ) | ||
|
|
||
|
|
||
| def create_hardware_backed_soft_signal( |
There was a problem hiding this comment.
Could: Make it match soft_r /soft_rw ?
| def create_hardware_backed_soft_signal( | |
| def create_r_hardware_backed_soft_signal( |
olliesilvester
left a comment
olliesilvester
left a comment
There was a problem hiding this comment.
Looks good, just a few minor comments and questions
The device standards page in the docs refer to create_hardware_backed_soft_signal, so we should change that too
| @pytest.mark.parametrize( | ||
| "create_hardware_func", | ||
| [ | ||
| create_r_hardware_backed_soft_signal, | ||
| do_nothing_on_write_rw_hardware_backed_signal, | ||
| ], |
There was a problem hiding this comment.
Could: use pytest request instead of the repeated pytest.mark.parametrize
Co-authored-by: olliesilvester <122091460+olliesilvester@users.noreply.github.com>
|
I've addressed some comments but given bluesky/ophyd-async#661 is close to complete and will replace this I don't want to spend too much more time on it. |
3 participants
Fixes #1077
Instructions to reviewer on how to test:
Checks for reviewer
dodal connect ${BEAMLINE}