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 text message port cli option #710

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

Conversation

digitaldisarray
Copy link

What did you change?

I added a command line option '--textport' that lets you send text to an arbitrary port number.

Why did you change it?

When developing a new meshtastic firmware module, there isn't a super easy way to send arbitrary text packets to a module. Previously, module developers that just want to receive text data for testing purposes would likely make their module receive normal text messages (port 1) during development which is a bit of a hack-y solution. Or, developers would have to leave the comfort of their command line and write a script to send the messages on the custom port.

Screenshot

screenshot-edited

@thebentern
Copy link
Contributor

IMO this should be limited PRIVATE_APP. We don't need to easily allow folks to send arbitrary payloads over common portnums like position and telemetry for instance

@digitaldisarray
Copy link
Author

That makes sense. I'll modify my PR when I have the time to just be for PRIVATE_APP.

@ianmcorvidae
Copy link
Contributor

Agreed on the proposed change. I've set off the CI build in case it brings up anything else that could use improvement, for whenever you get back to it.

@digitaldisarray
Copy link
Author

Thank you for your patience. I believe the changes I just made limits users to PRIVATE_APP in a reasonable way. Let me know if any other changes should be made. I'll get to them quicker this time :)

@ianmcorvidae
Copy link
Contributor

This looks good to me, sorry for the delay in re-checking it. Assuming that CI doesn't complain about anything, I'll get this merged soon.

@ianmcorvidae
Copy link
Contributor

Ah, spoke too soon. I guess it's got some issues with syntax, possibly needs some quote-shuffling. I'll try to take a look when I get chance if you don't first.

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