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

Add message protobuf #497

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Add message protobuf #497

wants to merge 12 commits into from

Conversation

ndnestor
Copy link

What does this PR do?

Creates needed protobuf changes for firmware PR #3784. It adds a message protobuf which represents a message stored in storage or memory and removes the DeviceState protobuf's rx_text_message as it becomes obsolete.

Checklist before merging

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

meshtastic/deviceonly.proto Outdated Show resolved Hide resolved
/*
* The sending node's short name.
*/
string sender_short_name = 1;
Copy link
Member

Choose a reason for hiding this comment

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

Please use the nodenum instead

Copy link
Author

Choose a reason for hiding this comment

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

This field is used for displaying a device's name on the screen. I don't think it'd be user-friendly to show the device number on the screen rather than the short name.

Copy link
Member

Choose a reason for hiding this comment

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

i was looking at it from a database point of view, and the short name is not a good way to provide a unique key. Maybe i don'tr fully get how the link to the user entries is made, especially if it is a DM

Copy link
Author

Choose a reason for hiding this comment

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

No link to the user entry is made given that that's not needed to display the name.

meshtastic/message.proto Show resolved Hide resolved
meshtastic/message.proto Show resolved Hide resolved
@ndnestor ndnestor marked this pull request as ready for review June 1, 2024 00:49
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