Support flashing .hex files onto any suitable External flash partition.

[hydra]

This allows uploading not only firmware, but system, osd logos/fonts and config backups too.
Additionally, the firmware partition is not always the 3rd partition on new FCs, other partitions can preceed it.

A partition is 'suitable' if it matches the length of the .hex file.

[haslinghuis]

Please solve the code smells too.

[haslinghuis]

@hydra you forgot some code smells, I have pointed them out to make it easier.

[hydra]

@haslinghuis I hadn't looked at the code-smells yet, the previous push was fixing the build failure. My IDE doesn't have an option for removing trailing spacing on Javascript files (it does on C/JAVA files...) Missed one, just forced pushed again to fix the other issues.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.6% Duplication

[haslinghuis]

@hydra which hardware is needed to test this - the merging is automated - and we need to test the PR before we can apply the tested label.

[hydra]

@haslinghuis SPRacingH7 EXTREME/ZERO/NANO/RF/EXTREME-PX4-EDITION/CINE.

I tested it on 3 of the above, each with different flash chips and DFU descriptors. Previously only EXTREME/NANO/ZERO worked.

[asizon]

@hydra these advantages looks so good, can you suply to our Configurator Maintainers with some hardware samples to can test all your new features?

[hydra]

Pretty sure the maintainers already have the SPRacingH7EXTREME, I recall sending a few out.

[asizon]

@hydra we are now new maintainers for the new Configurator maintaining system:
@haslinghuis
@limonspb
@chmelevskij
@asizon

[schugabe]

Flashing bf to a SPRacingH7RF works with this PR

[McGiverGim]

What is this? A bug in the FC? I don't understand how this works (the flash system). I suppose this cannot be fixed in any way and this is a workaround in the Configurator to support it?

[McGiverGim]

I close this because the only change was the commit, so we are already supporting this.

[hydra]

What additional testing is required in order to get the 'Tested' label added?

[haslinghuis]

@limonspb which hardware did you test with?
@hydra without an engineering sample we are not able to test this.

[limonspb]

@haslinghuis I haven't test anything. I saw @schugabe tested, figured that would be enough. Apologize, I remove the tag.

[asizon]

To make good quality merges the code should be tested by maintainers with the needed hardware.

[limonspb]

Gotcha!

[blckmn]

AUTOMERGE: (FAIL)

• github identifies PR as mergeable -> PASS
• assigned to a milestone -> PASS
• cooling off period lapsed -> PASS
• commit count less or equal to three -> PASS
• Don't merge label NOT found -> PASS
• at least one RN: label found -> PASS
• Tested label found -> FAIL
• assigned to an approver -> FAIL
• approver count at least three -> PASS

[hydra]

if the current maintainers do not have the hardware then perhaps the old maintainers should send their hardware to them?

As there is a global shortage of CPUs there are no new FCs that can be sent out.

Is it not enough that the designer of the hardware and the person that initially added support for flashing to external flash chips, and other users, have tested it? The updated code is isolated from non external flash chips too.

[mikeller]

What is the reason for matching this by size? This seems to be contrary to the way Intel Hex files are designed to work, and not really scalable / extensible:

• what happens if there are multiple partitions of the same size;
• what happens if the hex file is shorter than the partition it should go into;
• why is this not using the address, which is the way Intel Hex is meant to work.

[hydra]

1. Currently first matching partition is used, by design. There are no targets like this anyway.
2. Short hex results in no match, intended. All current EXST HEX images are the same size as the partition.
3. I agree! The pattern of checking the size come from the non-external flash code. To be honest the whole size checking should probably just be deleted since the next step of the dfu flashing processes deals with the actual addresses. If it was up to me I would ditch .hex files and use a zipped .json file and improve the entire release and flashing process and allow multiple images/memory areas to be flashed/updated. similar to how PX4 does it. I'll be happy to delete the size checking code from both the internal and external flash use cases and make it rely on addresses only, however this would take some time and I've got other outstanding work right now. Suggest merging this stop-gap solution until time allows for further improvements.

[hydra]

With regard to 3 above. This section can be deleted (mostly):

betaflight-configurator/src/js/protocols/stm32usbdfu.js

Lines 572 to 629 in 26ec8fb

	if (typeof chipInfo.internal_flash !== "undefined") {
	// internal flash
	self.chipInfo = chipInfo;
	self.flash_layout = chipInfo.internal_flash;
	self.available_flash_size = self.flash_layout.total_size - (self.hex.start_linear_address - self.flash_layout.start_address);
	GUI.log(i18n.getMessage('dfu_device_flash_info', (self.flash_layout.total_size / 1024).toString()));
	if (self.hex.bytes_total > self.available_flash_size) {
	GUI.log(i18n.getMessage('dfu_error_image_size',
	[(self.hex.bytes_total / 1024.0).toFixed(1),
	(self.available_flash_size / 1024.0).toFixed(1)]));
	self.cleanup();
	} else {
	self.getFunctionalDescriptor(0, function (descriptor, resultCode) {
	self.transferSize = resultCode ? 2048 : descriptor.wTransferSize;
	console.log('Using transfer size: ' + self.transferSize);
	self.clearStatus(function () {
	self.upload_procedure(1);
	});
	});
	}
	} else if (typeof chipInfo.external_flash !== "undefined") {
	// external flash, exst images must match the size of the partition being flashed.
	//
	// since the actual addresses to be flashed are in the .hex file this is not ideal
	// really we just need to make sure the flash chip contains addresses/offsets that match the ones in the .hex file.
	self.chipInfo = chipInfo;
	self.flash_layout = chipInfo.external_flash;
	self.available_flash_size = 0;
	// Find a partition that matches the size of the hex.
	for (const firmwareSectors of self.flash_layout.sectors) {
	const firmwarePartitionSize = firmwareSectors.total_size;
	if (firmwarePartitionSize === self.hex.bytes_total) {
	self.available_flash_size = firmwarePartitionSize;
	break;
	}
	}
	GUI.log(i18n.getMessage('dfu_device_flash_info', (self.flash_layout.total_size / 1024).toString()));
	if (self.available_flash_size === 0) {
	GUI.log(i18n.getMessage('dfu_error_image_mismatch'));
	self.cleanup();
	} else {
	self.getFunctionalDescriptor(0, function (descriptor, resultCode) {
	self.transferSize = resultCode ? 2048 : descriptor.wTransferSize;
	console.log('Using transfer size: ' + self.transferSize);
	self.clearStatus(function () {
	self.upload_procedure(2); // no option bytes to deal with
	});
	});
	}

This section could could be improved (out of scope of this PR):

betaflight-configurator/src/js/protocols/stm32usbdfu.js

Lines 777 to 804 in 26ec8fb

	// find out which pages to erase
	var erase_pages = [];
	for (var i = 0; i < self.flash_layout.sectors.length; i++) {
	for (var j = 0; j < self.flash_layout.sectors[i].num_pages; j++) {
	if (self.options.erase_chip) {
	// full chip erase
	erase_pages.push({'sector': i, 'page': j});
	} else {
	// local erase
	var page_start = self.flash_layout.sectors[i].start_address + j * self.flash_layout.sectors[i].page_size;
	var page_end = page_start + self.flash_layout.sectors[i].page_size - 1;
	for (var k = 0; k < self.hex.data.length; k++) {
	var starts_in_page = self.hex.data[k].address >= page_start && self.hex.data[k].address <= page_end;
	var end_address = self.hex.data[k].address + self.hex.data[k].bytes - 1;
	var ends_in_page = end_address >= page_start && end_address <= page_end;
	var spans_page = self.hex.data[k].address < page_start && end_address > page_end;
	if (starts_in_page || ends_in_page || spans_page) {
	var idx = erase_pages.findIndex(function (element, index, array) {
	return element.sector == i && element.page == j;
	});
	if (idx == -1)
	erase_pages.push({'sector': i, 'page': j});
	}
	}
	}
	}
	}

As you can see, all the addresses in the .hex are checked to see if they match an 'erasable page'. Instead of checking the size which I fully agree is wrong for both internal an external flash chips as the hex can contain non-contiguous memory addresses, the code could be updated to show 'address not found' or 'address not writable' error messages instead.

[hydra]

I'm closing this PR. I'll do another one that ditches the size checking for both internal and external flashes and relies on addresses alone which is a sensible long-term solution.