Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix ffc-4p flash #847

Open
wants to merge 1 commit into
base: UI-test-tools-Production-gen2
Choose a base branch
from
Open

fix ffc-4p flash #847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion batch/eeprom/DD2090_R5M1E5_oak_ffc_4p.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
"productName": "OAK-FFC-4P",
"boardCustom": "",
"hardwareConf": "F0-FV00-BC001",
"boardOptions": 3,
"boardOptions": 0,
"version": 7
}
2 changes: 1 addition & 1 deletion batch/oak_ffc_4p.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"title": "OAK FFC-4P",
"description": "OAK FFC-4P",
"image": "images/oak_d_ffc_4p.jpg",
"test_type": "OAK-FFC-4P",
"test_type": "OAK-FFC-4P-NOR",
"variants": [
{
"title":"OAK-FFC 4P (DD2090 R4M1E4)",
Expand Down
2 changes: 1 addition & 1 deletion test_ui.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1155,7 +1155,7 @@ def connexion_slot(self, signal):
self.print_logs('Writing EEPROM...')
eeprom_success, eeprom_msg, eeprom_data = self.flash_eeprom(device)
eeprom_written = True
elif 'FFC' in test_type:
elif ('FFC' in test_type) and ('NOR' not in test_type):
self.update_bootloader()
with dai.Device() as device:
self.print_logs("Writing EEPROM... ")
Comment on lines +1158 to 1161
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, it seems like device's EEPROM won't be flashed?

What if we fall out of elif cases? Add a final else to catch any sort of an issue in downselecting the test type

Copy link
Contributor Author

@daxoft daxoft Nov 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@themarpe for "else" case we don't need to write bootloader (for example for oak-d-lite and oak-1), should we add an else: pass? or another elif for oak-d-lite and oak-1; and an else with a rise exception/error print? The eeprom is written if it wasn't already here but yes, we can move this in a single place since we're doing for all cases, my bad.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daxoft

I see, so its written afterwards then.

One thing bothering me is parsing this test_type. IMO should act like an enum.. out of scope for now.

I don't get the change currently and the code organization is odd here. So if there is an NOR in test type we do NOT want to flash bootloader? If anything I'd expect we would, perhaps not flash for USB cases where just having the USB header would do.

On the contrary, how is this one handled then? https://github.com/luxonis/depthai/blob/8d07c760112f8aa24d36c10d9af4229d79479fe4/batch/oak_ffc_1p.json

Being OAK-FFC-1P will it flash BL? Which one? USB or ETH?, seems like its actually a PoE board? (atleast variant suggests so)
etc...

So going forward with this... the reason I don't like this approach is that EXISTING code has to be tweaked to handle some new device instead of a new code path being created. Each time such a change is made there is a non-zero chance that the test suite fails for a previously working device. We don't want to write tests for tests so preferably we retain as much code as possible if nothing changes. I'd much rather see the json describing which path to choose and how to configure it, and if we have some new tweak, there is always a new path opened which never applies to other existing devices that also don't add such an option. Eg an object test_setup passed along, which is then used to quickly create such tweaks and additonal code paths as needed. And test_type to be handled as an enum.

Expand Down