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

Merged
merged 3 commits into from
Feb 19, 2025

Conversation

digitaldisarray
Copy link
Contributor

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
Contributor 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
Contributor 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.

Copy link

codecov bot commented Feb 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.59%. Comparing base (03ac322) to head (3954fbd).
Report is 91 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #710      +/-   ##
==========================================
+ Coverage   60.29%   60.59%   +0.29%     
==========================================
  Files          24       24              
  Lines        3816     4032     +216     
==========================================
+ Hits         2301     2443     +142     
- Misses       1515     1589      +74     
Flag Coverage Δ
unittests 60.59% <100.00%> (+0.29%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ianmcorvidae ianmcorvidae merged commit 985366c into meshtastic:master Feb 19, 2025
11 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.

3 participants