rework port output command handling #159
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Debenben
force-pushed
the
portoutputcommand
branch
2 times, most recently
from
7db34cd to
815231a
Compare
use explicit queues for port output commands; this helps the device to remain responsive by preventing old port output commands from clogging the bluetooth transmission. add improved interrupt property; if true, ALL previous commands are discarded if false, NONE of the previous commands are discarded this changes the previous behavior where commands queued for transmission could not be discarded and transmitted commands would not wait for the device to complete the previous command. send commands to queue by default (except break and stop); this keeps and improves the ability to accept commands faster than they can be transmitted via bluetooth and ensures that no commands discard previous commands in case they are not explicitly chained. provide feedback about success of commands.
use new PortOutputSleep instead of timer; * the promise returned by rampPower and rampBrightness gets resolved if the command is interrupted. * rampPower and rampBrightness can no longer be altered by other commands while in progress. Other commands now queue after the ramp command or interrupt and stop its execution.
feedback and interrupts must not resolve commands which are being transmitted
Debenben
force-pushed
the
portoutputcommand
branch
from
cc5f74a to
5a86ee7
Compare
|
@nathankellenicki Thank you for merging some pull requests! For me this mechanism has been working well, I think it would be a benefit for the official library, too. I rebased the pull request, let me know if you have some questions, suggestions or would like me to do some changes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Initially I only planned to do some small improvements to prevent my port input commands from clogging up such that they were already outdated when being transmitted. But the whole undertaking got more and more inconsistent and messy, so I ended up doing a large-scale rework for all port output commands.
A large majority of code using the library should be compatible with the changes, but some things would change. Rough overview of most important breaking change without paying attention to detail:
same behavior:
motorA.rotateByDegrees(30).then(motorA.rotateByDegrees(30));Result: motorA rotates by 60 degrees in total
await motorA.rotateByDegrees(30); await motorA.rotateByDegrees(30);Result: motorA rotates by 60 degrees in total
motorA.rotateByDegrees(30); motorB.rotateByDegrees(30);Result: motorA and motorB start rotating at the same time by 30 degrees each
motorA.setAccelerationTime(300); motorA.rotateByDegrees(30);Result: the acceleration time is set to 300 ms and then motorA rotates by 30 degrees
different behavior:
motorA.rotateByDegrees(30); motorA.rotateByDegrees(30);Result old: motorA rotates by 30 degrees
Result new: motorA rotates by 60 degrees
motorA.rampPower(0,100,5000).then(res => console.log("feedback is " + res)); motorA.rotateByDegrees(30);Result old: motorA rotates by 30 degrees
Result new: motorA ramps power from 0 to 100, then prints "feedback is 34", then rotates by 30 degrees
new feature:
motorA.setAccelerationTime(300, true).then(res => console.log("feedback1 is " + res)); motorA.rotateByDegrees(30, 100, true).then(res => console.log("feedback2 is " + res));Result new: prints "feedback1 is 68" and discards the acceleration time command, then rotates by 30 degrees with speed 100, then prints "feedback2 is 34"
I tested with a technic hub and two motors so far it works great for me and I did not encounter any problems, therefore I decided to put this change up for discussion even though not every device has been tested.