Methods for setting/removing fixed position#802
Conversation
|
@bergie is attempting to deploy a commit to the Meshtastic Team on Vercel. A member of the Team first needs to authorize it. |
| latitude: number, | ||
| longitude: number, | ||
| ): Promise<number> { | ||
| const setPositionMessage = create(Protobuf.Admin.AdminMessageSchema, { |
There was a problem hiding this comment.
So this PR is great. Unfortunately, there is some undocumented functionality in the firmware that isn't being accounted for in this PR, its important these be added in the same PR.
The firmware wont accept a new fixed position if one is already set, which should to be handled in another method (single responsibility). You are required to unset the existing fixed position prior to setting a new fixed position, so we'll need a updateFixedPosition method or named something else should be added in order to handle this use case.
There was a problem hiding this comment.
Are you sure about that? I appears to me that just updating the position by a periodic setFixedPosition works
There was a problem hiding this comment.
The Python client library implementation seems quite similar to ours, except that they support setting an altitude:
https://github.com/meshtastic/python/blob/2b10459db05e8a897e689c84b30ad145a25d2512/meshtastic/node.py#L794
They also have a removeFixedPosition, though. Should we follow suit?
There was a problem hiding this comment.
I'm also not seeing the logic mentioned in firmware itself. But maybe I'm not looking in the right place?
https://github.com/meshtastic/firmware/blob/093a37a2b0888934a1941bc3f9c525e80255b5da/src/modules/AdminModule.cpp#L368
There was a problem hiding this comment.
Needing to clear the fixed position first is also mentioned in meshtastic/firmware#5747 (comment)
But as said, I'm so far not seeing the firmware code that would require this to happen.
There was a problem hiding this comment.
I'll request someone confirm this assumption in the #contributor-lounge chat.
There was a problem hiding this comment.
I think adding removeFixedPosition makes sense.
There was a problem hiding this comment.
I added that, so this should be functionally equivalent to the Python library
There was a problem hiding this comment.
There are set and remove fixed position admin message values, you have to remove the old value and disable the Boolean before setting a new value.
There was a problem hiding this comment.
Both methods are implemented here. But as discussed here and on Discord, so far I haven't seen anything requiring removing the old value before setting new in the actual firmware
danditomaso
left a comment
There was a problem hiding this comment.
Good PR, left a comment
|
@bergie this looks like it's ready to go, but can you run |
573f4e4 to
cb40fd2
Compare
Done. Had to rebase to get the correct formatter dependency. |
Description
Provides a method for setting a fixed position on the connected Meshtastic device. Useful for devices without GPS.
Will help with #87
Testing Done
Method was ported from signalk-meshtastic.