Skip to content

Compress NodeInfoLite bools into a bitfield #668

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jasonbcox
Copy link

What does this PR do?

Related Issue Meshtastic#6427

This PR proposes to squash 3 bytes into 1 and allows up to 5 new boolean flags "for free." This will save 2 bytes per NodeInfoLite, or about 200 - 500 bytes in memory for most devices.

I'm new to this, so the process for updating the protobufs and then refactoring the firmware is not clear to me. Please let me know if there is something else I must do before applying the associated refactor to the firmware repo.

Checklist before merging

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

@jasonbcox
Copy link
Author

I forgot to revisit switching out the uint32 for something smaller (since protobuf doesn't support uint8) before submitting, so the patch does not do as the PR claims. I will fix that up some time tomorrow.

@thebentern
Copy link
Contributor

I forgot to revisit switching out the uint32 for something smaller (since protobuf doesn't support uint8) before submitting, so the patch does not do as the PR claims. I will fix that up some time tomorrow.

You can use the nanopb annotations in the .options file to set the max size to 8. There should be some other examples in there where we do this for certain fields.

I like the approach here, but one concern I have is existing data migrations. When the NodeDB is first loaded from the file system and decoded by nanopb, I'm not sure that the existing field values wouldn't be simply lost in the deserialization process.

@jasonbcox jasonbcox force-pushed the nodeinfolite-bitfield branch from 2d25ac5 to 27d456b Compare March 28, 2025 14:10
@jasonbcox
Copy link
Author

I will check on the deserialization issue.

@thebentern
Copy link
Contributor

FWIW, I believe a more obvious change with much greater savings to consider is limiting User.long_name to a more reasonable length of 20 instead of 40, which I suspect will deserialize fine, just truncating the excess length. @garthvh and I were just discussing this change on Discord.

@mskvortsov
Copy link

JFYI

dep .bss sym bytes used
libpax seen_ids_map 8192
bsec2 workBuffer 4096

12K = roughly 70 NodeInfoLites

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.

3 participants