Prepare refactoring of notifications (removal of NamedInts) #2630
Conversation
…ating BOLT_PAIRING_ERRORS enums
…hem "unrollable" and first try to use mock.Mock()
|
@MattHag Can you review this PR? I invited you into the project at a level that I think allows you to be a suggested reviewer. |
|
Yes |
|
@MattHag OK, I set you up as a reviewer. |
There was a problem hiding this comment.
Looks good, but I have done some clean up and would to merge it right away.
Please enable this and I can merge it, afterwards
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork
| serial_reply = b"\x00\x00\x00\x00\x00\x00\x00\x00" | ||
| low_level_mock.read_register = mock.Mock(return_value=serial_reply) | ||
|
|
||
| receiver: Receiver = Receiver(MockLowLevelInterface(), None, {}, True, None, None) |
There was a problem hiding this comment.
mock.Mock() could be sufficient here, that's what I meant. But that's not so important and easy to change later on.
There was a problem hiding this comment.
Nevermind, it doesn't work with Mock alone.
| @@ -55,29 +63,41 @@ def process(device, notification): | |||
| return _process_device_notification(device, notification) | |||
|
|
|||
|
|
|||
| def _process_receiver_notification(receiver, n): | |||
| def _process_receiver_notification(receiver: "Receiver", hidpp_notification: "HIDPPNotification") -> Optional[bool]: | |||
There was a problem hiding this comment.
You don't need strings here, when you import the modules for type checking, instead you add
from __future__ import annotations
and the following will work
def _process_receiver_notification(receiver: Receiver, hidpp_notification: HIDPPNotification) -> bool | None:
It should already be activated. Is reviewer == maintainer ? Did you already made the changes or shall I do it? |
|
I already did, let me try how this update works :) |
|
Ok I will tell you if my computer starts smoking because of you ;) |
|
Seems like I still can't push it. Here is the updated version |
|
Shall we abandon this PR and PR the new branch then? |
|
Yes, if you don't mind. |
|
Now managed in https://github.com/MattHag/Solaar/tree/rloutrel/master |
#2273 : First 2 tests + add typing
Sorry, I could not understand how to use mock.Mock.
This could be merged already or wait for actual re-factoring that I will try to do next week.