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

Optional properties #545

Merged
merged 2 commits into from
Aug 8, 2024
Merged

Conversation

ianmcorvidae
Copy link
Contributor

What does this PR do?

This sets a bunch of stuff as optional, which allows us to distinguish between a true zero value (for e.g. temperature), encoded as usual with a zero, from an absent value, left out from the encoding.

I am guessing a little bit with lots of this. I'd love for stuff like hops_away to be able to have this set, so we can distinguish in clients between "old firmware" and "actually 0 hops", and similarly for channel numbers (unset channel on a packet sent to the radio meaning "firmware decides", set-to-0 meaning "actually use channel 0"). Some of that might also necessitate firmware changes, but this is the first step for that in any case.

The stuff that I'm pretty sure is good is places like the environmental telemetry, since those are the clearest cases of "this thing could actually be zero".

One other thing I did, which may make whitespace-ignorant diff viewing advisable, is that I ran dos2unix on telemetry.proto, since it's the only file we have with carriage returns in it. I separated that into a different commit, though, so it might also work to just look at the commit with the actual meat of things.

Checklist before merging

  • All top level messages commented
  • All enum members have unique descriptions

ianmcorvidae

This comment was marked as outdated.

@ianmcorvidae
Copy link
Contributor Author

Okay, force-pushed this to a much smaller scope. Still left in the position & waypoint stuff (and curious if we think that should also get pulled back), plus the telemetry changes which are the clearest benefit.

@thebentern thebentern merged commit bd9b1ab into meshtastic:2.5 Aug 8, 2024
1 of 2 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.

2 participants