Skip to content
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

change device mode after feedback only #145

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Debenben
Copy link
Contributor

@Debenben Debenben commented Jan 9, 2022

During a couple of different tests, I came across the following problem, here illustrated in a reproducible way for motor rotation:

const motorA = await hub.waitForDeviceAtPort("A");
motorA.requestUpdate(); // requestUpdate for default mode 0x00
motorA.on("rotate", () => { // motorA._mode is changed to 0x02
    console.log("recieved rotate");
});
// update for mode 0x00 arrives, parsing as mode 0x02 throws ERR_OUT_OF_RANGE

For my TechnicMediumHub this change is working perfectly and fixes the problem. I would recommend to test it with some different hub before merging though.

I am not happy about device._mode now having a setter function even though it is still labelled as readonly property, and also the names of the new function in general, but could not come up with better ones so far.

@nathankellenicki
Copy link
Owner

Thanks for this. I need to do a bit more testing with this so I'll do some this week with different hub types. I recognise the original issue though, good find.

device._mode is used to interpret incoming port value messages.
When changing the mode on a hub, the recieved port values are for
the old mode until the port input format message with the new mode
has arrived.
@Debenben
Copy link
Contributor Author

@nathankellenicki Did you encounter any issues with this? For me this change works well and was sufficient to solve all issues.

If you are unsure, one could add a mechanism with a timeout to request the mode again if no response is recieved and additionally ignore messages that do not have the expected size.

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.

None yet

2 participants