Add Channel Hash Utility to Node class #843
Conversation
### Summary
- Added a new method `get_channels_with_hash()` to the `Node` class.
- This method returns a list of dictionaries, each containing the channel index, role, name, and a hash value derived from the channel name and PSK.
- The hash is calculated using the existing `generate_hash` utility, ensuring consistency with other parts of the codebase.
- This utility makes it easier to programmatically access channel metadata, including a unique hash, for scripting, debugging, or integration purposes.
### Motivation
- The protobuf `Channel` objects do not include a hash value by default.
- This addition provides a Pythonic, easy-to-use way to access channel info and a unique hash for each channel, which can be useful for diagnostics, scripting, or UI display.
### Example Usage
```python
channels = node.get_channels_with_hash()
for ch in channels:
print(f"Index {ch['index']}: {ch['role']} name='{ch['name']}' hash={ch['hash']}")
```
### Impact
- No breaking changes; existing APIs and CLI output remain unchanged.
- The new method is additive and can be used where needed for enhanced channel introspection.
ianmcorvidae
left a comment
There was a problem hiding this comment.
A few things I'd like to see moved around or amended but in general I think this looks like a good addition. Thanks for this!
meshtastic/node.py
Outdated
| result ^= char | ||
| return result | ||
|
|
||
| def generate_hash(name: str, key: str) -> int: |
There was a problem hiding this comment.
I think this looks good, but a few thoughts:
- this function,
xor_hash, and a variable for the default key (asbytes, I think, rather than the base64 version) really all belong inmeshtastic.utilrather than here. - There's multiple forms of hashing in firmware so this should be named to denote that, perhaps
channel_hash. If we later want to add the frequency-slot-style hash, better if it's distinguished better from the start. - we should be more permissive with the function signature; both arguments should accept
Union[str, bytes]and in the function body we can useisinstanceto determine if they need translation via something like:
if isinstance(name, str):
name = bytes(name, "utf-8")
if isinstance(key, str):
key = base64.b64decode(key.replace("-", "+").replace("_", "/").encode("utf-8"))
if len(key) == 1:
key = default_key[:-1] + keyHopefully these changes sound good to you. If possible I'd also love to see some tests for these functions added but I know I've already made several requests :)
There was a problem hiding this comment.
I have some changes in as requested, looking at tests now something like ? (I didnt fully compute)
def test_channel_hash():
"""Test channel_hash"""
assert channel_hash(b"123") == 48
assert channel_hash(b"") == 0
def test_generate_channel_hash(data: bytes) -> int:
"""test a channel hash with testname: LongFast and test PSK: AQ=="""
assert generate_channel_hash(b"LongFast", b"\x01") == 108There was a problem hiding this comment.
Yeah, that sort of thing, though it could probably be more in-depth than that, ideally even using the hypothesis library to do some property-based testing (but that's a bit complex to explain in a GitHub comment). I've got a little bit of time here, so I'll try to add some of these things while doing things already here.
There was a problem hiding this comment.
ad04c26 threw in a bunch here if you're curious what I was thinking of. Should be good to merge in a sec though, as long as CI doesn't complain again for some reason.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #843 +/- ##
==========================================
+ Coverage 60.10% 60.16% +0.05%
==========================================
Files 24 24
Lines 4269 4295 +26
==========================================
+ Hits 2566 2584 +18
- Misses 1703 1711 +8
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:
|
this function, xor_hash, and a variable for the default key (as bytes, I think, rather than the base64 version) really all belong in meshtastic.util rather than here. There's multiple forms of hashing in firmware so this should be named to denote that, perhaps channel_hash. If we later want to add the frequency-slot-style hash, better if it's distinguished better from the start.
…d generate_channel_hash
Summary
get_channels_with_hash()to theNodeclass.generate_hashutility, ensuring consistency with other parts of the codebase.Motivation
Channelobjects do not include a hash value by default.Example Usage
Impact