-
Notifications
You must be signed in to change notification settings - Fork 352
[Optional] ClientSide Text Message Logging #986
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?
Changes from all commits
f8990e5
5bdf749
b978ce1
62ccd39
7283c9e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,6 +66,8 @@ | |
| from meshtastic.protobuf import admin_pb2, channel_pb2, clientonly_pb2, config_pb2, portnums_pb2, mesh_pb2 | ||
| from meshtastic.version import get_active_version | ||
|
|
||
| from meshtastic.message_store import MessageSettings, MessageStore, MessageLog, print_messages | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| # Map dotted preference paths to the protobuf enum that defines their flags. | ||
|
|
@@ -600,6 +602,16 @@ def onConnected(interface): | |
| onResponse=interface.getNode(args.dest, False, **getNode_kwargs).onAckNak, | ||
| portNum=portnums_pb2.PortNum.PRIVATE_APP if args.private else portnums_pb2.PortNum.TEXT_MESSAGE_APP | ||
| ) | ||
|
|
||
| #Save Outbound Messages -- if save enabled | ||
| if mt_config.message_store: | ||
| #Donot Save on Private | ||
| if not args.private: | ||
| mt_config.message_store.log_sent( | ||
| args.sendtext, | ||
| destination_id=args.dest, | ||
| channel=channelIndex, | ||
| ) | ||
| else: | ||
| meshtastic.util.our_exit( | ||
| f"Warning: {channelIndex} is not a valid channel. Channel must not be DISABLED." | ||
|
|
@@ -1642,6 +1654,18 @@ def common(): | |
| mt_config.logfile = logfile | ||
|
|
||
| subscribe() | ||
|
|
||
| #Display Messages | ||
| if handleShowMessagesArgs(args): | ||
| return | ||
|
|
||
| # (after the early arg checks, before any interface is constructed) | ||
| # Enable? StoreMessages? Call to get bool --> True (Save) | ||
| message_store = None | ||
| if handleMessageStoreArgs(args): | ||
| message_store = MessageStore(MessageLog()) | ||
| mt_config.message_store = message_store # so onConnected can reach it | ||
|
|
||
| if args.ble_scan: | ||
| logger.debug("BLE scan starting") | ||
| for x in BLEInterface.scan(): | ||
|
|
@@ -1806,6 +1830,78 @@ def common(): | |
| # don't call exit, background threads might be running still | ||
| # sys.exit(0) | ||
|
|
||
| def handleMessageStoreArgs(args) -> bool: | ||
| """Resolve whether message logging is enabled for this run. | ||
|
|
||
| Reads --messages, updates the saved setting for on/off, and returns | ||
| the effective logging state. Does not attach hooks or exit. | ||
| """ | ||
|
|
||
| args = mt_config.args | ||
| mode = args.messages | ||
|
|
||
| store = MessageSettings() | ||
|
|
||
| # No flag: use the saved setting (defaults to off if missing/corrupt) | ||
| if mode is None: | ||
| return store.is_enabled() | ||
|
|
||
| #Flag Messages ON: True - Creates MessageStore(MessageLog) to Log Messages | ||
| if mode == "on": | ||
| if not store.set_enabled(True): # persist -- if write to config fails | ||
| print("Message logging is on for this run only (could not save the setting).") | ||
| return True | ||
|
|
||
| if mode == "off": | ||
| if not store.set_enabled(False): # persist -- if write to config fails -- throws an err | ||
| meshtastic.util.our_exit( | ||
| "Error: could not save the 'off' setting, so message logging " | ||
| "may still be ON next time. Check permissions on ~/.meshtastic and retry.", | ||
| 1 | ||
| ) | ||
| return False | ||
|
|
||
| if mode == "live": | ||
| # Log for this run only; saved setting is untouched | ||
| return True | ||
|
|
||
| #Check the current status from the config | ||
| if mode == "status": | ||
| saved = store.is_enabled() | ||
| print(f"Message logging is {'ON' if saved else 'OFF'}") | ||
| return saved | ||
|
|
||
| return False # unreachable with argparse choices, but safe | ||
|
Comment on lines
+1833
to
+1874
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '1645,1710p;1820,1910p' meshtastic/__main__.py
git log --oneline be366660828b5a54469e703209a10aa954927791..7283c9ea0ab4653637b5f4d0e7d15f688688f883 -- meshtastic/__main__.py | head -30Repository: meshtastic/python Length of output: 6694 🏁 Script executed: #!/bin/bash
set -o pipefail
printf '%s\n' '--- PR commits ---'
git log --oneline --reverse be366660828b5a54469e703209a10aa954927791..7283c9ea0ab4653637b5f4d0e7d15f688688f883
printf '%s\n' '--- Relevant commit metadata ---'
git show --no-patch --format=fuller 5bdf749
printf '%s\n' '--- Relevant fix commit diff ---'
git show --format= --find-renames 5bdf749 -- meshtastic/__main__.py
printf '%s\n' '--- Current CLI definitions and startup call sites ---'
rg -n -C 5 'def common|def addMessageArgs|handleShowMessagesArgs|handleMessageStoreArgs|add_argument\([^\n]*messages|dest|parser\.get_default|mt_config\.parser' meshtastic/__main__.py | head -240
printf '%s\n' '--- Current main/parser region ---'
rg -n 'def main|ArgumentParser|parse_args' meshtastic/__main__.py | tail -40
printf '%s\n' '--- Current startup and helper code ---'
sed -n '1550,1905p' meshtastic/__main__.py
printf '%s\n' '--- Current parser construction ---'
sed -n '1905,2150p' meshtastic/__main__.pyRepository: meshtastic/python Length of output: 39189 🏁 Script executed: #!/bin/bash
set -o pipefail
printf '%s\n' '--- Relevant hunks in later PR commits ---'
for rev in b978ce1 62ccd39 7283c9e; do
printf '\n### %s\n' "$rev"
git show --format= --unified=8 "$rev" -- meshtastic/__main__.py |
rg -n -C 8 'handleMessageStoreArgs|handleShowMessagesArgs|args\.messages|parser\.get_default|message_store|Display Messages' || true
done
printf '%s\n' '--- Relevant code at each PR revision ---'
for rev in f8990e5 5bdf749 b978ce1 62ccd39 7283c9e; do
printf '\n### %s\n' "$rev"
git show "$rev:meshtastic/__main__.py" |
rg -n -C 7 'handleMessageStoreArgs|handleShowMessagesArgs|if handleMessageStoreArgs|mode == "status"|mode == "live"'
done
printf '%s\n' '--- Message CLI parser declaration ---'
sed -n '2485,2535p' meshtastic/__main__.py
printf '%s\n' '--- Parser construction and defaults ---'
sed -n '2700,2795p' meshtastic/__main__.py
printf '%s\n' '--- CLI documentation references ---'
rg -n --glob '*.md' -- '--messages|show-messages' . || trueRepository: meshtastic/python Length of output: 16791 🏁 Script executed: #!/bin/bash
printf '%s\n' '--- MessageSettings binding and implementation ---'
rg -n -C 8 'class MessageSettings|def set_enabled|def is_enabled|MessageSettings' meshtastic
printf '%s\n' '--- Parser defaults relevant to the proposed guard ---'
rg -n -C 5 'seriallog|--port|--messages' meshtastic/__main__.py | head -180
printf '%s\n' '--- Connection setup locations ---'
rg -n -C 3 'SerialInterface\(|TCPInterface\(|onConnected\(client\)' meshtastic/__main__.pyRepository: meshtastic/python Length of output: 11214 Return before connecting for setting-only
🐛 Suggested fix@@
args = mt_config.args
parser = mt_config.parser
+ setting_only_message_mode = (
+ args.messages in {"on", "off", "status"}
+ and all(
+ name == "messages" or value == parser.get_default(name)
+ for name, value in vars(args).items()
+ )
+ )
@@
# (after the early arg checks, before any interface is constructed)
# Enable? StoreMessages? Call to get bool --> True (Save)
- message_store = None
- if handleMessageStoreArgs(args):
+ messages_enabled = handleMessageStoreArgs(args)
+ if setting_only_message_mode:
+ return
+ message_store = None
+ if messages_enabled:
message_store = MessageStore(MessageLog())🤖 Prompt for AI Agents |
||
|
|
||
| def handleShowMessagesArgs(args) -> bool: | ||
| """Handle --show-messages. Returns True if it did, so the caller can stop | ||
| before any store is created or any connection is attempted.""" | ||
|
|
||
| args = mt_config.args | ||
| parser = mt_config.parser | ||
|
|
||
| mode = getattr(args, "show_messages", None) | ||
|
|
||
| if mode is None: | ||
| return False | ||
|
|
||
| # Local-only command: refuse anything else that was actually set. | ||
| # Compare each arg to its parser default so unset flags don't count. | ||
| allowed = {"show_messages", "dest"} #Exclude dest -- which is set @BRDCST_ADDR | ||
| others = [ | ||
| "--" + name.replace("_", "-") | ||
| for name, value in vars(args).items() | ||
| if name not in allowed and value != parser.get_default(name) | ||
| ] | ||
| if others: | ||
| meshtastic.util.our_exit( | ||
| "Error: --show-messages can't be combined with: " + ", ".join(sorted(others)), 1 | ||
| ) | ||
|
|
||
| if mode == "all": | ||
| print_messages() | ||
| return True | ||
|
|
||
|
|
||
| def addConnectionArgs(parser: argparse.ArgumentParser) -> argparse.ArgumentParser: | ||
| """Add connection specification arguments""" | ||
|
|
@@ -2396,6 +2492,43 @@ def addRemoteAdminArgs(parser: argparse.ArgumentParser) -> argparse.ArgumentPars | |
|
|
||
| return parser | ||
|
|
||
|
|
||
| def addMessageArgs(parser: argparse.ArgumentParser) -> argparse.ArgumentParser: | ||
| """Add message logging / history arguments.""" | ||
| group = parser.add_argument_group( | ||
| "Messages", | ||
| "Persistent text-message logging (client-side JSONL). " | ||
| ) | ||
|
|
||
| group.add_argument( | ||
| "--messages", | ||
| nargs="?", | ||
| const="status", | ||
| default=None, | ||
| choices=["on", "off", "status", "live"], | ||
| metavar="on|off|status|live", | ||
| help=( | ||
| "on/off: enable or disable message logging (saved) (default off). " | ||
| "status: show whether logging is on/off. " | ||
| "live: log messages for this run only, without changing the saved " | ||
| "setting. Combine with --listen or other commands." | ||
| ) | ||
| ) | ||
|
|
||
| group.add_argument( | ||
| "--show-messages", | ||
| nargs="?", | ||
| const="all", | ||
| default=None, | ||
| choices=["all"], | ||
| help=( | ||
| "Print stored messages. Currently supports: all." | ||
| ) | ||
| ) | ||
|
|
||
| return parser | ||
|
|
||
|
|
||
| def initParser(): | ||
| """Initialize the command line argument parsing.""" | ||
| parser = mt_config.parser | ||
|
|
@@ -2436,6 +2569,9 @@ def initParser(): | |
| parser = addRemoteActionArgs(parser) | ||
| parser = addRemoteAdminArgs(parser) | ||
|
|
||
| #Arguments for Message Parsing | ||
| parser = addMessageArgs(parser) | ||
|
|
||
| # All the rest of the arguments | ||
| group = parser.add_argument_group("Miscellaneous arguments") | ||
|
|
||
|
|
||
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.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: meshtastic/python
Length of output: 10295
🏁 Script executed:
Repository: meshtastic/python
Length of output: 41600
🏁 Script executed:
Repository: meshtastic/python
Length of output: 32458
Bind the message store to the client before sending.
If no receive packet arrives before
onConnectedhandles--sendtext,log_sent()storesfrom_id="self"andmy_id=None. A lookup by the sender’s node ID can miss the sent record, and its display omits that ID._on_receivebinds on any receive packet, not only text packets. The"self"fallback can remain for cases where the sender ID is unavailable; bind the connected client before the send path.Suggested fix
🤖 Prompt for AI Agents