-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow shared parent mock to be passed to Device.connect #599
base: signal-typing
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks about right. As you say a bunch of tests confirming all the use cases would be great
Would have to call attach_mock after each child device is connected as that is when the signal backend gets set to MockSignalBackend and the _backend.put_mock can be accessed. Tom has said he may touch how that is handled soon so I will wait for that to be done first. |
#594 is sufficiently advanced that you can work on top of it to see if you can apply this on top of it |
src/ophyd_async/core/_device.py
Outdated
for child_name, child_device in self.children(): | ||
# if mock is Mock instance, get child Mock if exists | ||
# else pass down boolean | ||
child_mock = getattr(mock, child_name, True) if mock else mock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mock should always be a Mock
here by now
child_mock = getattr(mock, child_name, True) if mock else mock | |
child_mock = getattr(mock, child_name) if mock else mock |
I've rebased on top of #594, still a WIP but it appears to work with the existing tests. I will work on porting over the new tests I added in the force-pushed-over commits here |
f749124
to
2785615
Compare
|
||
from ._device import Device |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is mostly in a sensible state now, depending on if we're happy with get_mock in _mock_signal_utils accepting Device | Signal
return _get_mock_signal_backend(device).mock | ||
return device.mock | ||
|
||
|
||
def reset_mock_put_calls(signal: Signal): | ||
backend = _get_mock_signal_backend(signal) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And this would be:
return _mock_signal_backends[signal]
src/ophyd_async/core/_device.py
Outdated
def __hash__(self): | ||
# to allow Devices to be used as dict keys | ||
return hash(id(self)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Devices are already hashable...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right! Will remove that now
src/ophyd_async/core/_device.py
Outdated
def __hash__(self): # to allow DeviceVector to be used as dict keys | ||
return hash(id(self)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But you're right that we need this to make DeviceVector hashable
Addresses #336
Allows a Device to accept a unittest.Mock or a boolean.
If True passed, creates a new Mock()
if Mock() passed, finds any child mocks set with attach_mock if they exist, else passes down mock=True to children.
Will push some tests for this
edit: now realising that I misunderstood the requirements, will fix quickly!