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

StoreForward updates #3194

Merged
merged 11 commits into from
Feb 14, 2024
Merged

StoreForward updates #3194

merged 11 commits into from
Feb 14, 2024

Conversation

GUVWAF
Copy link
Member

@GUVWAF GUVWAF commented Feb 10, 2024

Already does something using the iOS app, but it currently puts a * before each message:
image

GUVWAF and others added 5 commits February 10, 2024 12:55
- Send history in "text" variant
- Don't send history the client already got
- Check if PSRAM is full
- More sensible defaults
Use a mixture of full refresh, partial refresh, and skipped updates, balancing urgency and display health.

Co-authored-by: Ben Meadors <[email protected]>
@CLAassistant
Copy link

CLAassistant commented Feb 11, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
4 out of 5 committers have signed the CLA.

✅ GUVWAF
✅ thebentern
✅ todd-herbert
✅ garthvh
❌ github-actions[bot]
You have signed the CLA already but the status is still pending? Let us recheck it.

@GUVWAF GUVWAF marked this pull request as ready for review February 11, 2024 13:47
@GUVWAF
Copy link
Member Author

GUVWAF commented Feb 11, 2024

@garthvh @andrekir With the latest commit, when the Router sends its history, it will set the RequestResponse tag to ROUTER_TEXT_DIRECT for direct text messages and to ROUTER_TEXT_BROADCAST for broadcasts.

@andrekir
Copy link
Member

@garthvh @andrekir With the latest commit, when the Router sends its history, it will set the RequestResponse tag to ROUTER_TEXT_DIRECT for direct text messages and to ROUTER_TEXT_BROADCAST for broadcasts.

so we would need to request broadcasts or DMs separately?

@GUVWAF
Copy link
Member Author

GUVWAF commented Feb 11, 2024

@andrekir No, you can request the history of all packets with the CLIENT_HISTORY tag with history variant. The router then first responds with one ROUTER_HISTORY packet containing the amount of packets it is going to send in the field history_messages and the index of the last packet in last_request (which you can save and set in a subsequent request to avoid getting the same message twice even if the time window overlaps). Then it will send all the packets, which have either the TEXT_DIRECT or TEXT_BROADCAST tag.

thebentern added a commit to meshtastic/artifacts that referenced this pull request Feb 11, 2024
thebentern added a commit to meshtastic/artifacts that referenced this pull request Feb 11, 2024
@andrekir
Copy link
Member

@andrekir No, you can request the history of all packets with the CLIENT_HISTORY tag with history variant. The router then first responds with one ROUTER_HISTORY packet containing the amount of packets it is going to send in the field history_messages and the index of the last packet in last_request (which you can save and set in a subsequent request to avoid getting the same message twice even if the time window overlaps). Then it will send all the packets, which have either the TEXT_DIRECT or TEXT_BROADCAST tag.

sounds really good!

@garthvh
Copy link
Member

garthvh commented Feb 11, 2024

What does the router history do now? That is what I was sending before.

@GUVWAF
Copy link
Member Author

GUVWAF commented Feb 11, 2024

@garthvh I believe you were sending the CLIENT_HISTORY:
https://github.com/meshtastic/Meshtastic-Apple/blob/cd2d70c0563ba65a0d48b9d8c9ed1ff49427f0cb/Meshtastic/Helpers/BLEManager.swift#L2384
Which is good, that's the one you use to request the history, nothing changed there.

You'll only receive the ROUTER_HISTORY that has some informative fields like the number of messages the router is going to send.

Copy link
Member

@garthvh garthvh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all working on my end

thebentern added a commit to meshtastic/artifacts that referenced this pull request Feb 14, 2024
@thebentern thebentern merged commit d9bd9bd into meshtastic:master Feb 14, 2024
15 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.

6 participants