-
Notifications
You must be signed in to change notification settings - Fork 184
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
Waypoint creation/move/deletion #714
Conversation
Add methods to send (create or move), delete waypoint. Add an example script to create, move, delete waypoint.
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.
Thanks for the pull request! I've got a few questions and some things that appear to be missing. Would love to see this added to the API!
logging.debug(f"w.longitude_i:{w.longitude_i}") | ||
|
||
if wantResponse: | ||
onResponse = self.onResponseWaypoint |
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.
This function doesn't appear to exist.
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.
My bad, I forgot those indeed.
Latest commit add them but I am not sure how they should really behave from a protocol perspective. So please guide me if it is wrong.
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 functions are what'd normally be used by the CLI if it supported this stuff, but of course this PR doesn't include that. For now, I think they can just serve as placeholders, so what you have should be fine, I think.
channelIndex=channelIndex, | ||
) | ||
if wantResponse: | ||
self.waitForWaypoint() |
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.
This doesn't seem to exist either.
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.
see commit 7cc18e9
w.expire = expire | ||
if id is None: | ||
seed = secrets.randbits(32) | ||
w.id = math.floor(seed * math.pow(2, -32) * 1e9) |
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.
What is the source of this ID generation formula? Looking at the other clients, android seems to just use whatever would be the next packet ID and apple seems to generate a random one.
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 have done similar has in javascript API.
Note that it is not a packet ID but a waypoint ID.
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.
Fair enough. I'm not sure why restricting it to 9 digits is useful, but it's probably fine in any case for this applicaiton.
Add missing methods callbacks regarding waypoints.
I'm just running the CI checks on this to see if anything pops up there. As long as that's all fine, this seems good to merge. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #714 +/- ##
==========================================
+ Coverage 60.22% 60.35% +0.12%
==========================================
Files 24 24
Lines 3829 3970 +141
==========================================
+ Hits 2306 2396 +90
- Misses 1523 1574 +51
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Add
MeshInterface.sendWaypoint
to send (create or move) waypoint, andMeshInterface.deleteWaypoint
to delete one.Add an example script to create, move, delete waypoint.
MeshInterface.sendWaypoint
Example scripts usage looks like: