Refactor decode_devid to be callable by python code - #33882
Conversation
eea9e5a to
1e646ac
Compare
Also fix the lining issues and provide tests and examples
1e646ac to
d6bfcdb
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Refactors the decode_devid functionality into an importable Python module and adds scripts to demonstrate usage and validate parity with the original CLI output.
Changes:
- Added
decode_devid_lib.pyproviding reusable functions (parse_device_id,decode_device_id,get_device_type_name,format_device_info) plus a CLI entrypoint. - Added an example script demonstrating library usage (
example_usage_decode_devid.py). - Added a test script that compares outputs of
decode_devid.pyvsdecode_devid_lib.py(test_decode_devid.py).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| Tools/scripts/decode_devid_lib.py | Introduces importable decoding library + CLI wrapper to preserve script usability. |
| Tools/scripts/example_usage_decode_devid.py | Demonstrates how to call the new library functions from Python. |
| Tools/scripts/test_decode_devid.py | Adds parity tests comparing original script output to library output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if original_normalized == library_normalized: | ||
| self.tests_passed += 1 | ||
| status = "✓ PASS" | ||
| return True | ||
| else: | ||
| self.tests_failed += 1 | ||
| status = "✗ FAIL" | ||
| self.failures.append({ | ||
| 'description': description, | ||
| 'device_id': device_id_hex, | ||
| 'flag': device_type_flag, | ||
| 'original': original_output, | ||
| 'library': library_output, | ||
| }) | ||
| return False | ||
|
|
||
| print(f"{status} | {description}") | ||
| print(f" Device ID: {device_id_hex}, Flag: {device_type_flag}") | ||
| return original_normalized == library_normalized |
|
|
||
|
|
||
| def decode_device_id(device_id: int) -> Dict[str, any]: | ||
| """ |
|
|
||
|
|
||
| def format_device_info(decode_info: Dict, device_type_name: str = "") -> str: | ||
| """ |
| """Command-line interface.""" | ||
| parser = optparse.OptionParser("decode_devid.py") | ||
| parser.add_option("-C", "--compass", action='store_true', help='decode compass IDs') |
| """ | ||
| Example script demonstrating how to use decode_devid.py as an importable module. | ||
| """ |
| import sys | ||
| from pathlib import Path | ||
|
|
||
| # Add the current directory to path so we can import decode_devid |
|
|
||
| # Example: Decode a compass device ID | ||
| # Let's use a compass device with I2C bus | ||
| device_id = 0x00051001 # I2C bus, compass type 0x01 (HMC5883_OLD) |
peterbarker
left a comment
peterbarker
left a comment
There was a problem hiding this comment.
This doesn't look right - it's a sideways copy of the script.
If you want to make a library out of the thing then do it in place.
|
@peterbarker I just did a copy so that I can prove to you that it is working. That is what the test script does: compares the script with the lib. |
|
I will fix the pre-commit hooks in the next few days |
Summary
Refactor the decode_devid script to be callable by other python scripts
Also fix the lining issues and provide tests and examples
Classification & Testing (check all that apply and add your own)
Description
Here is the output of the
python .\example_usage_decode_devid.pyHere is the output of the
python .\test_decode_devid.pytest:I did break the _lib script on purpose and the test code found the issues.
Should it be merged like this or should I just change the original script?