mirror of
https://github.com/jkingsman/Remote-Terminal-for-MeshCore.git
synced 2026-11-03 08:02:19 +01:00
Implement better DM retry #41
Assignees
adam (Adam Wojcieszonek)
Clear assignees
adam (Adam Wojcieszonek)
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: adam/Remote-Terminal-for-MeshCore#41
Reference in new issue
Block a user
If we don't get an ack from a DM, attempt to retry in five seconds. Maybe make this toggleable? Or allow the question mark to be clicked to trigger a resend and/or open the ack modal to allow a resend? Automatic is probably smoother but I don't love a static retry that could be problematic on a many-hop connection where propagation is fine, but just takes longer. 5 seconds is probably reasonable; need to decide whether to resend byte-perfect (may not be re-repeated but means we can action any ack) or re-timestamped send (means dupe messages on longer hop distances may be sent before the first ack gets back.
My preference would be to resend byte-perfect; otherwise it might have made it out but just not acked (aka TX was fine, RX got clobbered).
How does the existing "Resend as new" dialog work today?
Sometimes I click on the "?" it shows "Resend" (byte-perfect I assume) and also "Resend as new". Other times it's only the "Resend as new" option.
I think byte-perfect is the right approach, from the docs it looks like if a path is currently known it will try 3x and then reset the path and send via flood.
ie
5.3. Q: What happens when a node learns a route via a mobile repeater, and that repeater is gone?
A: If you used to reach a node through a repeater and the repeater is no longer reachable, the client will send the message using the existing (but now broken) known path, the message will fail after 3 retries, and the app will reset the path and send the message as flood on the last retry by default. This can be turned off in settings. If the destination is reachable directly or through another repeater, the new path will be used going forward. Or you can set the path manually if you know a specific repeater to use to reach that destination.
In the case if users are moving around frequently, and the paths are breaking, they just see the phone client retries and revert to flood to attempt to re-establish a path.
Awesome; that's helpful info to know about the DM resend pattern; thank you!
So resend as byte-perfect means an exact clone, which repeaters nearby will drop if they have recently heard it. Increasing the timestamp means it will always go through as a new message, which is equivalent to retyping and resending, which is always legal.
I time byte-perfect out after 30 seconds because that's not really a "proper" resend pattern, and being able to drop byte perfect copies of quite-old packets onto the mesh felt like something disruptive. It's plausible that's how resends work already in the app, but I'm pretty sure they're incrementing the timestamp (plus, practically speaking, if your always-hop-1 repeater already heard the message, it's not gonna go any wider because that repeater will drop it as already-heard).
Okay so adding retry 3x + 4th time flood. I'll do that, and probably make the retries new-timestamp so you're not hamstrung by a repeater that already heard you.
Is this separate from the "native meshcore"
send_msg_with_retry()call?No, that's what I'm gonna use to do it; thanks for the poke. I probably shouldn't have tried to get fancy in the first place; that will cover everything we need.
I'm 99% sure you shouldn't let end-users muck with whatever values you end up choosing. Though making it a env variable or equiv might help dev testing... I know I'm always tempted to experiment but the average user probably shouldn't be given the power to cause unnecessary traffic congestion :)
Looks like you ended up with a different algorithm. Do you mind elaborating (the change got hard to follow)?
Basically it's a reimplementation of send_msg_with_retry. The downside of using that means we lose the ability to handle acks after the send timeout, or that happen after a reattempted send, since that function basically only returns "was acked" or "not acked" with nothing further, so no way to track future acks. The reimplementation means that we can accept those and use the ack from them. With some existing hairiness around tx delays that I'm seeing worked out in some other areas within MC, I want to be able to watch for acks that arrive later than expected, hence the parallel implementation.
I do still have a bug to resolve in #77.