Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Dynamic modes #97
base: master
Are you sure you want to change the base?
Dynamic modes #97
Changes from 6 commits
ea6b23b
ec37e6d
d4ea7cb
299963b
b4f1880
57c7baa
45b0c20
4edd5eb
8c451f2
8feffd6
4c429e2
c830cb9
df8ebd2
405b5f2
351bc90
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 haven't checked on all devices, but do all device modes only accept one type of value? Even for multiple values?
Are there any instances where a device could have ie. 3x int32, 1x uint8? I don't know if the UART protocol even supports that.
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 would say they mix everything as in the boost color and distance sensor:
For SPEC 1, which includes values from COLOR and PROX modes, it use 0-255 as range instead of 0-10 to match other values.
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.
As an example, the tilt sensor of the TechnicMediumHub has different data types for different modes in use (Int8, Int32, Int16) but within one mode, the data type is always the same. At least, that is what the device/spec is self-declaring.
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.
These are quite repetitive, and error prone when new devices are added (ie. could forget to do this). Could the check be done at hub level, calling either receive or autoParse?
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 am thinking about another way to do this:
Device.receive
to parse incoming messages (including mode information) and emit lego named eventsNo more
receive
method in specific device classes, just event handlers that could use private methods to handle mode calculation and formating.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 totally forgot about the subscription logic in the comment above. It will need the correct mode map.