-
Notifications
You must be signed in to change notification settings - Fork 4.9k
add D555 to test-fw-update #14428
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
add D555 to test-fw-update #14428
Conversation
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.
Pull Request Overview
This PR adds support for D555 devices to the firmware update test. The changes enable testing firmware updates for D555 devices alongside the existing D400 device support, including handling device-specific differences like the absence of an update counter in D555.
Key changes:
- Added D555 device support with a dedicated command-line argument for custom D555 firmware
- Refactored firmware selection logic to handle both D400 and D555 devices
- Updated device detection to use serial numbers for tracking devices through the update process
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| unit-tests/test-fw-update.py | Added D555 device support, refactored firmware path handling, improved device tracking with serial numbers, and added D555-specific logic for update counter |
| unit-tests/run-unit-tests.py | Added command-line argument support for custom D555 firmware path |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| print( ' --no-reset Do not try to reset any devices, with or without a hub' ) | ||
| print( ' --hub-reset If a hub is available, reset the hub itself' ) | ||
| print( ' --custom-fw-d400 If custom fw provided flash it if its different that the current fw installed' ) | ||
| print( ' --custom-fw-d555 If custom fw provided flash it if its different that the current fw installed' ) |
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.
Seems relevant to all D500
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.
done
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.
@OhadMeir we cannot use it for D500 as D555 and D585 does not share the same FW image.
Need to revert back to D555 only.
When D585 be added we will add another field for it
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 was under the impression that with D585 new FW update process uses a single file similar to the D555.
Can't the same procedure be used for both?
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.
run-unit-tests.py is called once for all cameras, same flow should work on D555 and D585, but the input file is different, so we need 2 parameters.
Does this answer?
If not lets talk :)
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.
As discussed, leave D555
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.
Reverted changes
unit-tests/test-fw-update.py
Outdated
|
|
||
| if product_line == "D400": | ||
| size = 0x2 | ||
| elif "D555" in product_name: |
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.
Same, check D500 product line, not D555 specific
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.
done
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.
That's OK to keep as all D500 does not have a counter
Co-authored-by: Copilot <[email protected]>
unit-tests/run-unit-tests.py
Outdated
| print( ' --no-reset Do not try to reset any devices, with or without a hub' ) | ||
| print( ' --hub-reset If a hub is available, reset the hub itself' ) | ||
| print( ' --custom-fw-d400 If custom fw provided flash it if its different that the current fw installed' ) | ||
| print( ' --custom-fw-d500 If custom fw provided flash it if its different that the current fw installed' ) |
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.
d555
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.
Reverted
unit-tests/run-unit-tests.py
Outdated
| longopts=['help', 'verbose', 'debug', 'quiet', 'regex=', 'stdout', 'tag=', 'list-tags', | ||
| 'list-tests', 'no-exceptions', 'context=', 'repeat=', 'retry=', 'config=', 'no-reset', 'hub-reset', | ||
| 'rslog', 'skip-disconnected', 'live', 'not-live', 'device=', 'exclude-device=', 'test-dir=','skip-regex=','custom-fw-d400='] ) | ||
| 'rslog', 'skip-disconnected', 'live', 'not-live', 'device=', 'exclude-device=', 'test-dir=','skip-regex=','custom-fw-d400=','custom-fw-d500='] ) |
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.
d555
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.
Reverted
unit-tests/test-fw-update.py
Outdated
| parser = argparse.ArgumentParser(description="Test firmware update") | ||
| parser.add_argument('--custom-fw-d400', type=str, help='Path to custom firmware file') | ||
| parser.add_argument('--custom-fw-d400', type=str, help='Path to custom D400 firmware file') | ||
| parser.add_argument('--custom-fw-d500', type=str, help='Path to custom D500 firmware file') |
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.
555
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.
Reverted
unit-tests/test-fw-update.py
Outdated
| custom_fw_version = None | ||
| if product_line == "D400" and args.custom_fw_d400: | ||
| custom_fw_path = args.custom_fw_d400 | ||
| elif product_line == "D500" and args.custom_fw_d500: |
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.
camera_name include D555
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.
Reverted
unit-tests/test-fw-update.py
Outdated
| image_file = find_image_or_exit(product_name) | ||
| # always flash signed fw when device on recovery before flashing anything else | ||
| # on D500 we currently do not have bundled FW | ||
| image_file = find_image_or_exit(product_name) if not product_line == "D500" else custom_fw_path |
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.
d555 all over
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.
All reverted
|
@AviaAv please avoid doing commit --amend during a CR as now we cannot see the new changed and need to review all changes again.. |
Tracked on: [LRS-1349]