When using the example for HTTPUpdate->HttpUpdate example I get the following error reported from the callbacks:
CALLBACK: HTTP update process started
CALLBACK: HTTP update fatal error code 0
HTTP_UPDATE_FAILD Error (0):
I assumed that a "fatal" error should have a valid reason other than 0 - UPDATE_ERROR_OK.
Turned on debug, saw that data was arriving and written to the underlying LittleFS file of "firmware.bin". Eventually found a cryptic message of lfs_write reported a -28. -28 in the lfs.h file is no space remaining.
In the Updater.cpp file I find the following call to LittleFS write
bool UpdaterClass::_writeBuffer() {
if (_command == U_FLASH) {
if (_bufferLen != _fp.write(_buffer, _bufferLen)) {
return false;
}
} else {
_fp.write is returning a value of 0, a value != to _bufferLen and takes the return false path . Returning false ripples up to a the failed download message but not reporting source of the error. Noticing similar checks for not sufficient flash space I suggest the following fix.
CALLBACK: HTTP update process at 49152 of 577508 bytes...
_fp.write returns 4096
CALLBACK: HTTP update process at 53248 of 577508 bytes...
_fp.write returns 0
CALLBACK: HTTP update fatal error code 4
HTTP_UPDATE_FAILD Error (4): Update error: ERROR[4]: Not Enough Space
To duplicate this problem one must send a sufficiently large update file that is bigger than the configured flash size. I set it to FS: 64kb.
In the process of verification of this fix, I configured the flash size for (no FS) and I get the same problem from a different location in the code also from Updater.cpp:
if (command == U_FLASH) {
LittleFS.begin();
_fp = LittleFS.open("firmware.bin", "w+");
if (!_fp) {
#ifdef DEBUG_UPDATER
DEBUG_UPDATER.println(F("[begin] unable to create file"));
#endif
return false;
}
updateStartAddress = 0; // Not used
} else if (command == U_FS) {
if (&_FS_start + size > &_FS_end) {
_setError(UPDATE_ERROR_SPACE);
return false;
}
which fails the file open test, optionally prints the DEBUG_UPDATER message and returns false. Same incorrect error code is reported. Adding the _setError(UPDATE_ERROR_SPACE); function call after the check for (!fp) fixes this error path too.
Unlike the ESP32 partition table, the pico does not currently have the ability to have separate spiffs and littlefs flash locations. Both U_FLASH and U_FS use the same flash memory locations. I think I saw in another discussion a mention that 2MB is barely sufficient as it is. Based on this assumption for the pico, the same test comparing FS_start + size > _FS_end test could be applied before attempting even the LittleFS.begin for the earliest possible error. For future proofing and the possibility of larger flash sizes and the addition of a partition table similar to ESP32, it might not be the best idea to add this particular test,
I've tested the following snippet with correct behaviour with all flash size variations.
if (command == U_FLASH) {
+ if (&_FS_start + size > &_FS_end) {
+ _setError(UPDATE_ERROR_SPACE);
+ return false;
+ }
LittleFS.begin();
_fp = LittleFS.open("firmware.bin", "w+");
if (!_fp) {
#ifdef DEBUG_UPDATER
DEBUG_UPDATER.println(F("[begin] unable to create file"));
#endif
+ _setError(UPDATE_ERROR_SPACE);
return false;
}
updateStartAddress = 0; // Not used
} else if (command == U_FS) {
if (&_FS_start + size > &_FS_end) {
_setError(UPDATE_ERROR_SPACE);
return false;
}
When using the example for HTTPUpdate->HttpUpdate example I get the following error reported from the callbacks:
````
CALLBACK: HTTP update process started
CALLBACK: HTTP update fatal error code 0
HTTP_UPDATE_FAILD Error (0):
````
I assumed that a "fatal" error should have a valid reason other than 0 - UPDATE_ERROR_OK.
Turned on debug, saw that data was arriving and written to the underlying LittleFS file of "firmware.bin". Eventually found a cryptic message of lfs_write reported a -28. -28 in the lfs.h file is no space remaining.
In the Updater.cpp file I find the following call to LittleFS write
````
bool UpdaterClass::_writeBuffer() {
if (_command == U_FLASH) {
if (_bufferLen != _fp.write(_buffer, _bufferLen)) {
return false;
}
} else {
````
_fp.write is returning a value of 0, a value != to _bufferLen and takes the return false path . Returning false ripples up to a the failed download message but not reporting source of the error. Noticing similar checks for not sufficient flash space I suggest the following fix.
````
bool UpdaterClass::_writeBuffer() {
if (_command == U_FLASH) {
if (_bufferLen != _fp.write(_buffer, _bufferLen)) {
_setError(UPDATE_ERROR_SPACE);
return false;
}
} else {
````
and the expected error message:
````
CALLBACK: HTTP update process at 49152 of 577508 bytes...
_fp.write returns 4096
CALLBACK: HTTP update process at 53248 of 577508 bytes...
_fp.write returns 0
CALLBACK: HTTP update fatal error code 4
HTTP_UPDATE_FAILD Error (4): Update error: ERROR[4]: Not Enough Space
````
To duplicate this problem one must send a sufficiently large update file that is bigger than the configured flash size. I set it to FS: 64kb.
In the process of verification of this fix, I configured the flash size for (no FS) and I get the same problem from a different location in the code also from Updater.cpp:
````
if (command == U_FLASH) {
LittleFS.begin();
_fp = LittleFS.open("firmware.bin", "w+");
if (!_fp) {
#ifdef DEBUG_UPDATER
DEBUG_UPDATER.println(F("[begin] unable to create file"));
#endif
return false;
}
updateStartAddress = 0; // Not used
} else if (command == U_FS) {
if (&_FS_start + size > &_FS_end) {
_setError(UPDATE_ERROR_SPACE);
return false;
}
````
which fails the file open test, optionally prints the DEBUG_UPDATER message and returns false. Same incorrect error code is reported. Adding the _setError(UPDATE_ERROR_SPACE); function call after the check for (!fp) fixes this error path too.
Unlike the ESP32 partition table, the pico does not currently have the ability to have separate spiffs and littlefs flash locations. Both U_FLASH and U_FS use the same flash memory locations. I think I saw in another discussion a mention that 2MB is barely sufficient as it is. Based on this assumption for the pico, the same test comparing FS_start + size > _FS_end test could be applied before attempting even the LittleFS.begin for the earliest possible error. For future proofing and the possibility of larger flash sizes and the addition of a partition table similar to ESP32, it might not be the best idea to add this particular test,
I've tested the following snippet with correct behaviour with all flash size variations.
````
if (command == U_FLASH) {
+ if (&_FS_start + size > &_FS_end) {
+ _setError(UPDATE_ERROR_SPACE);
+ return false;
+ }
LittleFS.begin();
_fp = LittleFS.open("firmware.bin", "w+");
if (!_fp) {
#ifdef DEBUG_UPDATER
DEBUG_UPDATER.println(F("[begin] unable to create file"));
#endif
+ _setError(UPDATE_ERROR_SPACE);
return false;
}
updateStartAddress = 0; // Not used
} else if (command == U_FS) {
if (&_FS_start + size > &_FS_end) {
_setError(UPDATE_ERROR_SPACE);
return false;
}
````
Thanks for the very detailed analysis and proposed solution. Your analysis looks good. Error checking and return values are always the second-to-last things I get around to (the last being documentation, of course! 😆 ). Good additional size check at the start, too. The RPI team did add partitions w/ROM support with the RP2350, but I've not looked into it and don't have any immediate plans to do so. If they are added, Updater.cpp will probably be the least trouble to fix...the OTA app and all filesystem and EEPROM and BT TLV will be where the pain is.
Would you like to make a PR so you get the credit in the git blame logs?
Thanks for the very detailed analysis and proposed solution. Your analysis looks good. Error checking and return values are always the second-to-last things I get around to (the last being documentation, of course! 😆 ). Good additional size check at the start, too. The RPI team did add partitions w/ROM support with the RP2350, but I've not looked into it and don't have any immediate plans to do so. If they are added, `Updater.cpp` will probably be the least trouble to fix...the OTA app and all filesystem and EEPROM and BT TLV will be where the pain is.
Would you like to make a PR so you get the credit in the `git blame` logs?
Sorry, I think I was about halfway into writing a response the other day and then got pulled away. I've been retired for about 3 years. We used git all the time at work but I haven't got any of the tools set up from home. Probably would have taken me twice the this amount of time just to get back up to speed.
Thank you for all the time you put into keeping pico and esp32 stuff running so well. It was very helpful to me when others found bugs in my code. One of these days soon I might just need to get those tools downloaded at home. Then I would be a little more helpful to you and others.
Sorry, I think I was about halfway into writing a response the other day and then got pulled away. I've been retired for about 3 years. We used git all the time at work but I haven't got any of the tools set up from home. Probably would have taken me twice the this amount of time just to get back up to speed.
Thank you for all the time you put into keeping pico and esp32 stuff running so well. It was very helpful to me when others found bugs in my code. One of these days soon I might just need to get those tools downloaded at home. Then I would be a little more helpful to you and others.
Don Small
Get Outlook for Android<https://aka.ms/AAb9ysg>
________________________________
From: Earle F. Philhower, III ***@***.***>
Sent: Tuesday, February 4, 2025 12:54:34 PM
To: earlephilhower/arduino-pico ***@***.***>
Cc: donmsmall ***@***.***>; Author ***@***.***>
Subject: Re: [earlephilhower/arduino-pico] OTA update fails with "fatal" error code 0 (Issue #2785)
Closed #2785<https://github.com/earlephilhower/arduino-pico/issues/2785> as completed via #2793<https://github.com/earlephilhower/arduino-pico/pull/2793>.
—
Reply to this email directly, view it on GitHub<https://github.com/earlephilhower/arduino-pico/issues/2785#event-16184400237>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BGEEZLLNTKAD5HY7ZUVTUVT2OELHVAVCNFSM6AAAAABWHJ6JIKVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJWGE4DINBQGAZDGNY>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
When using the example for HTTPUpdate->HttpUpdate example I get the following error reported from the callbacks:
I assumed that a "fatal" error should have a valid reason other than 0 - UPDATE_ERROR_OK.
Turned on debug, saw that data was arriving and written to the underlying LittleFS file of "firmware.bin". Eventually found a cryptic message of lfs_write reported a -28. -28 in the lfs.h file is no space remaining.
In the Updater.cpp file I find the following call to LittleFS write
_fp.write is returning a value of 0, a value != to _bufferLen and takes the return false path . Returning false ripples up to a the failed download message but not reporting source of the error. Noticing similar checks for not sufficient flash space I suggest the following fix.
and the expected error message:
To duplicate this problem one must send a sufficiently large update file that is bigger than the configured flash size. I set it to FS: 64kb.
In the process of verification of this fix, I configured the flash size for (no FS) and I get the same problem from a different location in the code also from Updater.cpp:
which fails the file open test, optionally prints the DEBUG_UPDATER message and returns false. Same incorrect error code is reported. Adding the _setError(UPDATE_ERROR_SPACE); function call after the check for (!fp) fixes this error path too.
Unlike the ESP32 partition table, the pico does not currently have the ability to have separate spiffs and littlefs flash locations. Both U_FLASH and U_FS use the same flash memory locations. I think I saw in another discussion a mention that 2MB is barely sufficient as it is. Based on this assumption for the pico, the same test comparing FS_start + size > _FS_end test could be applied before attempting even the LittleFS.begin for the earliest possible error. For future proofing and the possibility of larger flash sizes and the addition of a partition table similar to ESP32, it might not be the best idea to add this particular test,
I've tested the following snippet with correct behaviour with all flash size variations.
Thanks for the very detailed analysis and proposed solution. Your analysis looks good. Error checking and return values are always the second-to-last things I get around to (the last being documentation, of course! 😆 ). Good additional size check at the start, too. The RPI team did add partitions w/ROM support with the RP2350, but I've not looked into it and don't have any immediate plans to do so. If they are added,
Updater.cppwill probably be the least trouble to fix...the OTA app and all filesystem and EEPROM and BT TLV will be where the pain is.Would you like to make a PR so you get the credit in the
git blamelogs?Sorry, I think I was about halfway into writing a response the other day and then got pulled away. I've been retired for about 3 years. We used git all the time at work but I haven't got any of the tools set up from home. Probably would have taken me twice the this amount of time just to get back up to speed.
Thank you for all the time you put into keeping pico and esp32 stuff running so well. It was very helpful to me when others found bugs in my code. One of these days soon I might just need to get those tools downloaded at home. Then I would be a little more helpful to you and others.
Don Small
Get Outlook for Androidhttps://aka.ms/AAb9ysg
From: Earle F. Philhower, III @.>
Sent: Tuesday, February 4, 2025 12:54:34 PM
To: earlephilhower/arduino-pico @.>
Cc: donmsmall @.>; Author @.>
Subject: Re: [earlephilhower/arduino-pico] OTA update fails with "fatal" error code 0 (Issue #2785)
Closed #2785https://github.com/earlephilhower/arduino-pico/issues/2785 as completed via #2793https://github.com/earlephilhower/arduino-pico/pull/2793.
—
Reply to this email directly, view it on GitHubhttps://github.com/earlephilhower/arduino-pico/issues/2785#event-16184400237, or unsubscribehttps://github.com/notifications/unsubscribe-auth/BGEEZLLNTKAD5HY7ZUVTUVT2OELHVAVCNFSM6AAAAABWHJ6JIKVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJWGE4DINBQGAZDGNY.
You are receiving this because you authored the thread.Message ID: @.***>
No worries and thanks again for pointing out the problem and a solution!