On 2014-11-21 21:28, Anton Lundin wrote:
+dc_status_t +hw_ostc3_device_fwupdate (dc_device_t *abstract, const char *filename) +{
- dc_status_t rc = DC_STATUS_SUCCESS;
- hw_ostc3_device_t *device = (hw_ostc3_device_t *) abstract;
- dc_context_t *context = (abstract ? abstract->context : NULL);
- // Enable progress notifications.
- dc_event_progress_t progress = EVENT_PROGRESS_INITIALIZER;
- if (!ISINSTANCE (abstract))
return DC_STATUS_INVALIDARGS;- if (device->state == OPEN) {
if ((rc = hw_ostc3_device_service_mode(abstract)) !=DC_STATUS_SUCCESS)
return rc;- } else if (device->state != SERVICE) {
return DC_STATUS_INVALIDARGS;- }
- // load, erase, upload FZ, verify FZ, reprogram
- progress.maximum = 3 + SZ_FIRMWARE * 2 / SZ_FIRMWARE_BLOCK;
- device_event_emit (abstract, DC_EVENT_PROGRESS, &progress);
- hw_ostc3_device_display(abstract, " Loading FW...");
- // Allocate memory for the firmware data.
- hw_ostc3_firmware_t *firmware = (hw_ostc3_firmware_t *) malloc
(sizeof (hw_ostc3_firmware_t));
- if (firmware == NULL) {
ERROR (context, "Failed to allocate memory.");return DC_STATUS_NOMEMORY;- }
- // Read the hex file.
- rc = hw_ostc3_firmware_readfile (firmware, context, filename);
- if (rc != DC_STATUS_SUCCESS) {
free (firmware);return rc;- }
- // Firmware loaded
- progress.current++;
- device_event_emit (abstract, DC_EVENT_PROGRESS, &progress);
I think it's better to load the firmware file before trying to boot into service mode. If there is anything wrong with the firmware file, then there is no point to boot into service mode. That might save user some trouble.
Reading the firmware file should be instant compared to the time it takes to flash it. So the fact that we won't reporting any progress for this stage won't be an issue.
BTW, how do you exit service mode? Here, I'm thinking about the case where something goes wrong and we have to abort the firmware upgrade. If I understand correctly, the original firmware will be preserved, so the device can't be bricked. But I assume it's more re-assuring to the user if the device boots again, rather than staying in service mode. Note that I haven't tested the firmware update yet, so if this is something obvious just ignore the question :-)
- hw_ostc3_device_display(abstract, " Erasing FW...");
I'm not really sure what's the best way to deal with these notifications. When doing this from within libdivecomputer, then it won't be possible to localize these messages. That might be confusing because the ostc firmware is available in several languages. Doing this on the application is not impossible, but there you only have access to the progress events, so you could show the overall percentage but not the stage (e.g. erasing, uploading, verifying and flashing).
Suggestions are welcome.
rc = hw_ostc3_device_read_block (device, FIRMWARE_AREA + len,block, sizeof(block));
if (rc != DC_STATUS_SUCCESS || memcmp (firmware->data + len, block,sizeof (block)) != 0) {
hw_ostc3_device_display (abstract, " Verify FAILED");free (firmware);ERROR (context, "Failed verify.");return DC_STATUS_IO;}
If hw_ostc3_device_read_block() fails, you should return rc, like you already do elsewhere. Not DC_STATUS_IO. And when the memcmp() fails, the most appropriate error code is DC_STATUS_PROTOCOL.
Jef