-
Notifications
You must be signed in to change notification settings - Fork 233
add --set-is-unmessageable to CLI commands #794
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
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #794 +/- ##
=======================================
Coverage 59.42% 59.43%
=======================================
Files 24 24
Lines 4128 4156 +28
=======================================
+ Hits 2453 2470 +17
- Misses 1675 1686 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking this should be rearranged a little to match other code, but otherwise looks reasonable to me.
@@ -327,6 +327,23 @@ def setOwner(self, long_name: Optional[str]=None, short_name: Optional[str]=None | |||
onResponse = self.onAckNak | |||
return self._sendAdmin(p, onResponse=onResponse) | |||
|
|||
def setIsUnmessageable(self, is_unmessagable: Optional[bool]=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should probably add this to setOwner
, since we're already messing with all these fields there. And, it should be combined with set-owner and set-owner-short where those are present, which is probably done that way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! That makes a lot of sense. Thanks for the review, I'll revise.
@@ -350,6 +350,26 @@ def onConnected(interface): | |||
print(f"Setting device owner short to {args.set_owner_short}") | |||
interface.getNode(args.dest, False, **getNode_kwargs).setOwner(long_name=args.set_owner, short_name=args.set_owner_short) | |||
|
|||
if args.set_is_unmessageable: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly to the comment in node.py, this block should be merged into the if args.set_owner or args.set_owner_short
above it, so everything's done in one admin command to the node.
allows:
meshtastic --set-is-unmessagable true
meshtastic --set-is-unmessageable true
meshtastic --set-is-unmessagable false
meshtastic --set-is-unmessageable false