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

Waypoint creation/move/deletion #714

Merged
merged 3 commits into from
Dec 27, 2024

Conversation

loic-fejoz
Copy link
Contributor

Add MeshInterface.sendWaypoint to send (create or move) waypoint, and MeshInterface.deleteWaypoint to delete one.
Add an example script to create, move, delete waypoint.

MeshInterface.sendWaypoint

Example scripts usage looks like:

python3 examples/waypoint.py --port /dev/ttyUSB0 create 45 test "the description" "2024-12-18T23:05:23" 48.74 7.35
python3 examples/waypoint.py delete 45

Add methods to send (create or move), delete waypoint.
Add an example script to create, move, delete waypoint.
@CLAassistant
Copy link

CLAassistant commented Dec 18, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@ianmcorvidae ianmcorvidae left a 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
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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()
Copy link
Contributor

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.

Copy link
Contributor Author

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)
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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.
@ianmcorvidae
Copy link
Contributor

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.

Copy link

codecov bot commented Dec 27, 2024

Codecov Report

Attention: Patch coverage is 13.88889% with 62 lines in your changes missing coverage. Please review.

Project coverage is 60.35%. Comparing base (7c89e23) to head (57f0598).
Report is 32 commits behind head on master.

Files with missing lines Patch % Lines
meshtastic/mesh_interface.py 11.29% 55 Missing ⚠️
meshtastic/util.py 30.00% 7 Missing ⚠️
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     
Flag Coverage Δ
unittests 60.35% <13.88%> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ianmcorvidae ianmcorvidae merged commit f4c085f into meshtastic:master Dec 27, 2024
10 of 11 checks passed
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.

3 participants