fix ffc-4p flash #847
fix ffc-4p flash #847
Conversation
| elif ('FFC' in test_type) and ('NOR' not in test_type): | ||
| self.update_bootloader() | ||
| with dai.Device() as device: | ||
| self.print_logs("Writing EEPROM... ") |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
No description provided.