provision.sh.in: accept STM32_Programmer -otp displ failure

[olheureu]

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
  tim.anderson@foundries.io, 2023-02-09 10:26:32 -0700
• [2] "bash(1) — Linux manual page"
  https://www.man7.org/linux/man-pages/man1/bash.1.html

[Tim-Anderson]

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

[olheureu]

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 provision.sh use. At FORT Robotics, being able to re-provision an already-produced device is a requirement.
But as the already-provisioned devices will have a closed STM32MP15x, the OTP will be inaccessible.
If you add an error_exit in case the OTP is inaccessible, you forbids re-provisioning.

Reprovisioning a closed device was tested: it works perfectly, with a warning message that the OTP can't be accessed.

[olheureu]

It will then exit with an error at line 236 since the grep will exit with an error.

In line 236, the grep (part of stm32prog_state_is_open) is embedded in an if test
if stm32prog_state_is_open; then
and consequently does not force a script exit, even with set -e.

[olheureu]

To be completely clear: reprovisioning a closed device where the OTP is inaccessible is a feature.

[Tim-Anderson]

LGTM

[MrCry0]

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.

[olheureu]

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.

[MrCry0]

Thanks, it looks reasonable.

[ricardosalveti]

LGTM