From 7fcbbe9b80eaffb637af4b8d7cf962f346559876 Mon Sep 17 00:00:00 2001 From: Felix Moessbauer Date: Fri, 25 Oct 2024 14:12:23 +0200 Subject: [PATCH 1/7] refactor camel and snake naming in getPref We have a lot of code duplication by checkin over and over again if field should be named in camel or snake notation. We simplify this by writing the choosen variant into a variable and just use that name across the code. No functional change. --- meshtastic/__main__.py | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/meshtastic/__main__.py b/meshtastic/__main__.py index e7e9dbe1..a60be1d0 100644 --- a/meshtastic/__main__.py +++ b/meshtastic/__main__.py @@ -94,6 +94,7 @@ def getPref(node, comp_name): camel_name = meshtastic.util.snake_to_camel(name[1]) # Note: protobufs has the keys in snake_case, so snake internally snake_name = meshtastic.util.camel_to_snake(name[1]) + uni_name = camel_name if mt_config.camel_case else snake_name logging.debug(f"snake_name:{snake_name} camel_name:{camel_name}") logging.debug(f"use camel:{mt_config.camel_case}") @@ -112,14 +113,9 @@ def getPref(node, comp_name): break if not found: - if mt_config.camel_case: - print( - f"{localConfig.__class__.__name__} and {moduleConfig.__class__.__name__} do not have an attribute {snake_name}." - ) - else: - print( - f"{localConfig.__class__.__name__} and {moduleConfig.__class__.__name__} do not have attribute {snake_name}." - ) + print( + f"{localConfig.__class__.__name__} and {moduleConfig.__class__.__name__} do not have an attribute {uni_name}." + ) print("Choices are...") printConfig(localConfig) printConfig(moduleConfig) @@ -131,16 +127,8 @@ def getPref(node, comp_name): config_values = getattr(config, config_type.name) if not wholeField: pref_value = getattr(config_values, pref.name) - if mt_config.camel_case: - print(f"{str(config_type.name)}.{camel_name}: {str(pref_value)}") - logging.debug( - f"{str(config_type.name)}.{camel_name}: {str(pref_value)}" - ) - else: - print(f"{str(config_type.name)}.{snake_name}: {str(pref_value)}") - logging.debug( - f"{str(config_type.name)}.{snake_name}: {str(pref_value)}" - ) + print(f"{str(config_type.name)}.{uni_name}: {str(pref_value)}") + logging.debug(f"{str(config_type.name)}.{uni_name}: {str(pref_value)}") else: print(f"{str(config_type.name)}:\n{str(config_values)}") logging.debug(f"{str(config_type.name)}: {str(config_values)}") From 1abb9fb21304d5afa9a6b5c5fff7dcc136beea23 Mon Sep 17 00:00:00 2001 From: Felix Moessbauer Date: Fri, 25 Oct 2024 14:30:53 +0200 Subject: [PATCH 2/7] refactor getPref to use uniform printing logic We add a local helper function to print a single setting. This is a preparation to correctly print non-trivial types. The existing code in getPref is ported over to use that function. By that, the output of "wholeField" is changed slightly to always print the full path for each setting (e.g. "security.serialEnabled" instead of "security:\nserialEnabled"). This improves support for grepping on the output. --- meshtastic/__main__.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/meshtastic/__main__.py b/meshtastic/__main__.py index a60be1d0..0d267891 100644 --- a/meshtastic/__main__.py +++ b/meshtastic/__main__.py @@ -87,6 +87,10 @@ def checkChannel(interface: MeshInterface, channelIndex: int) -> bool: def getPref(node, comp_name): """Get a channel or preferences value""" + def _printSetting(config_type, uni_name, pref_value): + """Pretty print the setting""" + print(f"{str(config_type.name)}.{uni_name}: {str(pref_value)}") + logging.debug(f"{str(config_type.name)}.{uni_name}: {str(pref_value)}") name = splitCompoundName(comp_name) wholeField = name[0] == name[1] # We want the whole field @@ -114,7 +118,7 @@ def getPref(node, comp_name): if not found: print( - f"{localConfig.__class__.__name__} and {moduleConfig.__class__.__name__} do not have an attribute {uni_name}." + f"{localConfig.__class__.__name__} and {moduleConfig.__class__.__name__} do not have attribute {uni_name}." ) print("Choices are...") printConfig(localConfig) @@ -127,11 +131,10 @@ def getPref(node, comp_name): config_values = getattr(config, config_type.name) if not wholeField: pref_value = getattr(config_values, pref.name) - print(f"{str(config_type.name)}.{uni_name}: {str(pref_value)}") - logging.debug(f"{str(config_type.name)}.{uni_name}: {str(pref_value)}") + _printSetting(config_type, uni_name, pref_value) else: - print(f"{str(config_type.name)}:\n{str(config_values)}") - logging.debug(f"{str(config_type.name)}: {str(config_values)}") + for field in config_values.ListFields(): + _printSetting(config_type, field[0].name, field[1]) else: # Always show whole field for remote node node.requestConfig(config_type) From 839bbbcad299ca8fb0ba2e18cf1a76f0bcf02a0e Mon Sep 17 00:00:00 2001 From: Felix Moessbauer Date: Fri, 25 Oct 2024 14:50:25 +0200 Subject: [PATCH 3/7] config: correctly print byte and array types on get When getting config values of type bytes or list (technically a protobuf repeated container type), these were directly printed on the output. However, the retrieved values could not be set by --set again, as the format was different (e.g. python string representation of bytes vs. base64 prefixed and encoded as expected by --set). We fix this by adding a toStr utility function (similar to the fromStr) function to convert byte types correctly to the base64 representation. Further, we check if the type is repeated and apply this operation to all values. --- meshtastic/__main__.py | 12 +++++++++--- meshtastic/util.py | 7 +++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/meshtastic/__main__.py b/meshtastic/__main__.py index 0d267891..632bdd70 100644 --- a/meshtastic/__main__.py +++ b/meshtastic/__main__.py @@ -87,8 +87,12 @@ def checkChannel(interface: MeshInterface, channelIndex: int) -> bool: def getPref(node, comp_name): """Get a channel or preferences value""" - def _printSetting(config_type, uni_name, pref_value): + def _printSetting(config_type, uni_name, pref_value, repeated): """Pretty print the setting""" + if repeated: + pref_value = [meshtastic.util.toStr(v) for v in pref_value] + else: + pref_value = meshtastic.util.toStr(pref_value) print(f"{str(config_type.name)}.{uni_name}: {str(pref_value)}") logging.debug(f"{str(config_type.name)}.{uni_name}: {str(pref_value)}") @@ -131,10 +135,12 @@ def _printSetting(config_type, uni_name, pref_value): config_values = getattr(config, config_type.name) if not wholeField: pref_value = getattr(config_values, pref.name) - _printSetting(config_type, uni_name, pref_value) + repeated = pref.label == pref.LABEL_REPEATED + _printSetting(config_type, uni_name, pref_value, repeated) else: for field in config_values.ListFields(): - _printSetting(config_type, field[0].name, field[1]) + repeated = field[0].label == field[0].LABEL_REPEATED + _printSetting(config_type, field[0].name, field[1], repeated) else: # Always show whole field for remote node node.requestConfig(config_type) diff --git a/meshtastic/util.py b/meshtastic/util.py index 3f864a2c..45527fa8 100644 --- a/meshtastic/util.py +++ b/meshtastic/util.py @@ -100,6 +100,13 @@ def fromStr(valstr): return val +def toStr(raw_value): + """Convert a value to a string that can be used in a config file""" + if isinstance(raw_value, bytes): + return "base64:" + base64.b64encode(raw_value).decode("utf-8") + return str(raw_value) + + def pskToString(psk: bytes): """Given an array of PSK bytes, decode them into a human readable (but privacy protecting) string""" if len(psk) == 0: From 4c29d7dd0fe8b8d79110dcf3998a9526c9c831cf Mon Sep 17 00:00:00 2001 From: Felix Moessbauer Date: Fri, 25 Oct 2024 14:58:04 +0200 Subject: [PATCH 4/7] refactor camel and snake naming in setPref Same change as done in getPerf to have less branches (simplifies the code). --- meshtastic/__main__.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/meshtastic/__main__.py b/meshtastic/__main__.py index 632bdd70..1a7c2efe 100644 --- a/meshtastic/__main__.py +++ b/meshtastic/__main__.py @@ -177,6 +177,7 @@ def setPref(config, comp_name, valStr) -> bool: snake_name = meshtastic.util.camel_to_snake(name[-1]) camel_name = meshtastic.util.snake_to_camel(name[-1]) + uni_name = camel_name if mt_config.camel_case else snake_name logging.debug(f"snake_name:{snake_name}") logging.debug(f"camel_name:{camel_name}") @@ -213,14 +214,9 @@ def setPref(config, comp_name, valStr) -> bool: if e: val = e.number else: - if mt_config.camel_case: - print( - f"{name[0]}.{camel_name} does not have an enum called {val}, so you can not set it." - ) - else: - print( - f"{name[0]}.{snake_name} does not have an enum called {val}, so you can not set it." - ) + print( + f"{name[0]}.{uni_name} does not have an enum called {val}, so you can not set it." + ) print(f"Choices in sorted order are:") names = [] for f in enumType.values: @@ -255,10 +251,7 @@ def setPref(config, comp_name, valStr) -> bool: getattr(config_values, pref.name)[:] = cur_vals prefix = f"{'.'.join(name[0:-1])}." if config_type.message_type is not None else "" - if mt_config.camel_case: - print(f"Set {prefix}{camel_name} to {valStr}") - else: - print(f"Set {prefix}{snake_name} to {valStr}") + print(f"Set {prefix}{uni_name} to {valStr}") return True From 6ceae7c72f97202fe03d7b820e88eb98d44ca19b Mon Sep 17 00:00:00 2001 From: Felix Moessbauer Date: Fri, 25 Oct 2024 15:01:20 +0200 Subject: [PATCH 5/7] setPref: pass typed value instead of string By passing a typed value we can conserve the type information to later use that to convert it back into the correct protobuf type. The type conversion is now done inside setPref. --- meshtastic/__main__.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/meshtastic/__main__.py b/meshtastic/__main__.py index 1a7c2efe..8fc8a432 100644 --- a/meshtastic/__main__.py +++ b/meshtastic/__main__.py @@ -165,12 +165,12 @@ def traverseConfig(config_root, config, interface_config): if isinstance(config[pref], dict): traverseConfig(pref_name, config[pref], interface_config) else: - setPref(interface_config, pref_name, str(config[pref])) + setPref(interface_config, pref_name, config[pref]) return True -def setPref(config, comp_name, valStr) -> bool: +def setPref(config, comp_name, raw_val) -> bool: """Set a channel or preferences value""" name = splitCompoundName(comp_name) @@ -199,10 +199,13 @@ def setPref(config, comp_name, valStr) -> bool: if (not pref) or (not config_type): return False - val = meshtastic.util.fromStr(valStr) - logging.debug(f"valStr:{valStr} val:{val}") + if isinstance(raw_val, str): + val = meshtastic.util.fromStr(raw_val) + else: + val = raw_val + logging.debug(f"valStr:{raw_val} val:{val}") - if snake_name == "wifi_psk" and len(valStr) < 8: + if snake_name == "wifi_psk" and len(str(raw_val)) < 8: print(f"Warning: network.wifi_psk must be 8 or more characters.") return False @@ -237,7 +240,7 @@ def setPref(config, comp_name, valStr) -> bool: except TypeError: # The setter didn't like our arg type guess try again as a string config_values = getattr(config_part, config_type.name) - setattr(config_values, pref.name, valStr) + setattr(config_values, pref.name, str(val)) else: config_values = getattr(config, config_type.name) if val == 0: @@ -245,13 +248,13 @@ def setPref(config, comp_name, valStr) -> bool: print(f"Clearing {pref.name} list") del getattr(config_values, pref.name)[:] else: - print(f"Adding '{val}' to the {pref.name} list") + print(f"Adding '{raw_val}' to the {pref.name} list") cur_vals = [x for x in getattr(config_values, pref.name) if x not in [0, "", b""]] cur_vals.append(val) getattr(config_values, pref.name)[:] = cur_vals prefix = f"{'.'.join(name[0:-1])}." if config_type.message_type is not None else "" - print(f"Set {prefix}{uni_name} to {valStr}") + print(f"Set {prefix}{uni_name} to {raw_val}") return True From 4ca13bcede31b52c5f79b83e62fdcb05f3823860 Mon Sep 17 00:00:00 2001 From: Felix Moessbauer Date: Fri, 25 Oct 2024 15:48:55 +0200 Subject: [PATCH 6/7] fix setting of list item on configure When setting the whole configuration via --configure, list types like adminKey need special handling. Previously this failed as a list cannot be appended to a list. The new code adds dedicated logic to replace the repeated value when passing a list. Also, all items of that list are converted into the correct (typed) form before setting them. --- meshtastic/__main__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/meshtastic/__main__.py b/meshtastic/__main__.py index 8fc8a432..7aa9f5ed 100644 --- a/meshtastic/__main__.py +++ b/meshtastic/__main__.py @@ -241,6 +241,10 @@ def setPref(config, comp_name, raw_val) -> bool: # The setter didn't like our arg type guess try again as a string config_values = getattr(config_part, config_type.name) setattr(config_values, pref.name, str(val)) + elif type(val) == list: + new_vals = [meshtastic.util.fromStr(x) for x in val] + config_values = getattr(config, config_type.name) + getattr(config_values, pref.name)[:] = new_vals else: config_values = getattr(config, config_type.name) if val == 0: From 578d3e4b24bed09feca1e8d86afbb99201fe138c Mon Sep 17 00:00:00 2001 From: Felix Moessbauer Date: Fri, 25 Oct 2024 15:53:27 +0200 Subject: [PATCH 7/7] do not double-print value when setting a repeated value When clearning or appending to a repeated value, both the "Clearing..." / "Adding..." line and the "Set..." line were shown. However, this is misleading as the only performed operation is the clearing / appending. We fix this by directly returning from the function in case of clearing / appending. --- meshtastic/__main__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/meshtastic/__main__.py b/meshtastic/__main__.py index 7aa9f5ed..46038223 100644 --- a/meshtastic/__main__.py +++ b/meshtastic/__main__.py @@ -256,6 +256,7 @@ def setPref(config, comp_name, raw_val) -> bool: cur_vals = [x for x in getattr(config_values, pref.name) if x not in [0, "", b""]] cur_vals.append(val) getattr(config_values, pref.name)[:] = cur_vals + return True prefix = f"{'.'.join(name[0:-1])}." if config_type.message_type is not None else "" print(f"Set {prefix}{uni_name} to {raw_val}")