Position module should enforce precision for phone originated BROADCAST position packets

[thebentern]

No description provided.

[GUVWAF]

I'm afraid this only works for broadcasts and not for DMs because the packet from the phone comes in at PhoneAPI::handleToRadio() and then MeshService::handleToRadio() and eventually in Router::sendLocal() it calls handleReceived() (where the modules are called and thus packet will be altered) only when it's a broadcast:

firmware/src/mesh/Router.cpp

Lines 175 to 178 in 21311bb

	// If we are sending a broadcast, we also treat it as if we just received it ourself
	// this allows local apps (and PCs) to see broadcasts sourced locally
	if (p->to == NODENUM_BROADCAST) {
	handleReceived(p, src);

[thebentern]

I'm afraid this only works for broadcasts and not for DMs because the packet from the phone comes in at PhoneAPI::handleToRadio() and then MeshService::handleToRadio() and eventually in Router::sendLocal() it calls handleReceived() (where the modules are called and thus packet will be altered) only when it's a broadcast:

Good point. We generally only see position DMs in the case of the "Exchange positions" feature in the nodes list for the apps, right? Do you have any thoughts on a better place to dilute the precision? Hoping to avoid making changes at the end of the pipeline.

[GUVWAF]

In the iOS app you can also send your position in a DM.

Yeah, it's hard, I've not found a nice place to put this yet.

[thebentern]

In the iOS app you can also send your position in a DM.

Perhaps, this is a messaging issue we can just call out better? I could even make the case that when you explicitly DM a position to a node, you want the full precision.

[jp-bennett]

In the iOS app you can also send your position in a DM.

Perhaps, this is a messaging issue we can just call out better? I could even make the case that when you explicitly DM a position to a node, you want the full precision.

Yeah, I'd argue that a DM only incidentally uses a channel, and shouldn't use the channel's precision setting.

[garthvh]

In the iOS app you can also send your position in a DM.

Perhaps, this is a messaging issue we can just call out better? I could even make the case that when you explicitly DM a position to a node, you want the full precision.

Yeah, I'd argue that a DM only incidentally uses a channel, and shouldn't use the channel's precision setting.

Same

[jp-bennett]

Same

Though maybe with a warning for the security conscious, that it does re-use the channel password, so could be intercepted.

[thebentern]

We already have the caveat of DMs only being private in a sense within the channel. Not sure how / where we want to admonish the user.

[garthvh]

We already have the caveat of DMs only being private in a sense within the channel. Not sure how / where we want to admonish the user.

I can figure it out, I am the only client with this functionality currently.

[garthvh]

I like it

[thebentern]

@andrekir will this work on the Android app as well? I want to make sure I don't double dip on reducing precision for channel.

[andrekir]

@andrekir will this work on the Android app as well? I want to make sure I don't double dip on reducing precision for channel.

Android only sends position to the local node (phone GPS) with full precision, so should be good.

[garthvh]

Closes meshtastic/Meshtastic-Apple#617

[ianmcorvidae]

Glad to see this since I just added request position to Python too. I believe I shouldn't need to do anything special to get this? i.e. sending a regular want-response position packet with the full-precision will get diluted by this

[thebentern]

Glad to see this since I just added request position to Python too. I believe I shouldn't need to do anything special to get this? i.e. sending a regular want-response position packet with the full-precision will get diluted by this

Yes, if it's sent as a broadcast, it will be diluted based on the channel's precision. DMs will get full precision positions.

[ianmcorvidae]

Ah, okay, so this isn't terribly valuable for the request-positions type features, since those are rarely broadcast. Still potentially valuable for things that do get sent broadcast.

Maybe more relevant, though: I guess that the PositionModule also handles packets promiscuously, so (if I'm understanding right), those DM'd positions will still update the nodedbs for those on the same channel who aren't the destination of the DM. That might be an unintended effect here -- unsure?

[garthvh]

When you add persistence you need to make sure and only use node info, positions and messages that are DMs to update the appropriate node