provision.sh.in: accept STM32_Programmer -otp displ failure#1048
provision.sh.in: accept STM32_Programmer -otp displ failure#1048ricardosalveti merged 1 commit intofoundriesio:mainfrom
Conversation
The "provision.sh" script was made more robust by commit a9b949c [1] that introduced a "set -e" command in the Bash script [2], which: set -e Exit immediately if a pipeline (...), exits with a non-zero status. The script is more robust in the sense that it will exit immediately in case a subcommand fails. The purpose is to avoid to close an STM32MP15x from which the OTP is not fully burned. When testing commit a9b949c [1] on a closed STM32MP15x device, "provision.sh" stopped with an error message: Flashing service completed successfully + stm32prog_otp_refresh_values ++ STM32_Programmer_CLI -c port=USB1 -otp displ + otp_displ=' ------------------------------------------------------------------- STM32CubeProgrammer v2.12.0 ------------------------------------------------------------------- [...] Uploading OTP data: Error: Read OTP Partition failed Error: Uploading the OTP structure failed Error: Initializing the OTP structure failed ' The diagnostic is easy: on a closed STM32MP15x, the "STM32_Programmer_CLI -c port=USB1 -otp displ" command embedded in line 105: otp_displ=$(STM32_Programmer_CLI -c port=USB1 -otp displ) fails and return an error code 1, because the OTP is not accessible on closed STM32MP15x. With "set -e", this failure aborts the script. Fixed: accept an error on that particular command, by appending a "|| echo ..." to the command. References: - [1] commit a9b949c (recipes-support: provision.sh.in: add error exit handling) from Tim Anderson <[email protected]>, 2023-02-09 10:26:32 -0700 - [2] "bash(1) — Linux manual page" <https://www.man7.org/linux/man-pages/man1/bash.1.html> Signed-off-by: Olivier L'Heureux <[email protected]>
Tim-Anderson
left a comment
Tim-Anderson
left a comment
There was a problem hiding this comment.
Yes that will output a message but still continue. then at line 120 it should be an Error_exit since it should not progress if not OTP data is read.
It will then exit with an error at line 236 since the grep will exit with an error.
I suggest a graceful error_exit at line 120
It depends on the Reprovisioning a closed device was tested: it works perfectly, with a warning message that the OTP can't be accessed. |
In line 236, the |
|
To be completely clear: reprovisioning a closed device where the OTP is inaccessible is a feature. |
|
|
||
| stm32prog_otp_refresh_values () { | ||
| otp_displ=$(STM32_Programmer_CLI -c port=USB1 -otp displ) | ||
| otp_displ=$(STM32_Programmer_CLI -c port=USB1 -otp displ || echo "STM32_Programmer_CLI -otp displ returned an error") |
There was a problem hiding this comment.
I would change it to
otp_displ=$(STM32_Programmer_CLI -c port=USB1 -otp displ) || echo "STM32_Programmer_CLI -otp displ returned an error"
So that the error is shown to a user instead of appending to otp_displ variable.
There was a problem hiding this comment.
I have tested your proposal: it works with bash and dash. But I am not so sure it is want we want.
The purpose of the otp_displ=$(...) assignment is, in my view, to retrieve the information once, so that we can parse it and know the OTP state. In my view, a closed STM32MP15x with an unreadable OTP is something perfectly normal and supported. So I wanted something that just negates the code 1 returned by STM32_Programmer_CLI. I could have written something as || /usr/bin/true, but I think we could be more explicit with a sort of comment that would be written in the $otp_displ content. Those who inspect it will know more about the OTP state.
My intention was not to issue a warning, nothing is malfunctioning.
Your proposal will generate a visible warning when the STM32MP15x is closed. My code consider a closed STM32MP15x as a feature.
There was a problem hiding this comment.
Thanks, it looks reasonable.
4 participants
The "
provision.sh" script was made more robust by commit a9b949c [1] that introduced a "set -e" command in the Bash script [2], which:The script is more robust in the sense that it will exit immediately in case a subcommand fails. The purpose is to avoid to close an STM32MP15x from which the OTP is not fully burned.
When testing commit a9b949c [1] on a closed STM32MP15x device, "
provision.sh" stopped with an error message:The diagnostic is easy: on a closed STM32MP15x, the
"STM32_Programmer_CLI -c port=USB1 -otp displ" command embedded in line 105:otp_displ=$(STM32_Programmer_CLI -c port=USB1 -otp displ)fails and return an error code 1, because the OTP is not accessible on closed STM32MP15x. With "
set -e", this failure aborts the script.Fixed: accept an error on that particular command, by appending a "
|| echo ..." to the command.References:
error exit handling) from Tim Anderson
[email protected], 2023-02-09 10:26:32 -0700
https://www.man7.org/linux/man-pages/man1/bash.1.html