Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

west: flash: stm32cubeprogrammer: Reset command line argument is wrong #53487

Closed
nordicjm opened this issue Jan 4, 2023 · 4 comments
Closed

west: flash: stm32cubeprogrammer: Reset command line argument is wrong #53487

nordicjm opened this issue Jan 4, 2023 · 4 comments
Assignees
@nordicjm
Labels
area: Flash area: West West utility bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32

Comments

@nordicjm
Copy link
Collaborator

nordicjm commented Jan 4, 2023

Describe the bug
The tests for the stm32cubeprogrammer control reset by specifying --reset <mode>, however this is invalid, the actual argument is --reset-mode <mode> therefore one can assume that the tests are not properly checking this argument as if they were, the tests would be failing. @erwango I will leave it to you to decide if you think this is a deficiency in the tests that need adding, however this bug is just for fixing the argument itself (which is failing due to a PR I have)

Expected behavior
Command line argument should be correct (this bug)
Tests should actually validate the reset mode (not part of this bug)

Impact
Showstopper

Environment (please complete the following information):

  • Commit SHA or Version used: 58c924e
@nordicjm nordicjm added bug The issue is a bug, or the PR is fixing a bug area: Flash platform: STM32 ST Micro STM32 area: West West utility labels Jan 4, 2023
@nordicjm nordicjm self-assigned this Jan 4, 2023
@nordicjm nordicjm mentioned this issue Jan 4, 2023
Closed
nordicjm added a commit to nordicjm/zephyr that referenced this issue Jan 4, 2023
@nordicjm
west: runners: Add option to allow/prevent board reset
10b0ab7 
This adds supports for specifying if boards should be reset or not
after an image has been flashed, this is most useful for sysbuild
where multiple images are flashed and having one boot before all
are flashed causes the debug interface to be disabled or faults,
preventing correct operation of the firmware. Existing reset
priority of runners that supported it has been maintained.

This can be enabled per-board by creating a `flash_runner.yml` file
in the board directory with a list named `board_delay_reset`, e.g.
to delay an nRF5340 board from resetting whilst network core images
are being flashed, or whilst application core images (secure and
non-secure combined) are being flashed, the file contents would be
similar to:

board_delay_reset:
  - [
      nrf5340dk_nrf5340_cpuapp,
      nrf5340dk_nrf5340_cpuapp_ns,
    ]
  - [
      nrf5340dk_nrf5340_cpunet,
    ]

Also fixes zephyrproject-rtos#53487 by using the correct argument for the
stm32cubeprogrammer tests of "--reset-mode" instead of "--reset".

Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
@erwango
Copy link
Member

erwango commented Jan 4, 2023

The tests for the stm32cubeprogrammer control reset by specifying --reset <mode>

Can you verify ?

Here is a test sample (skipping parameters that are not of interest for current issue)

        "port": "swd",
        "frequency": None,
        "reset_mode": "core",
        [...]
        "calls": [
            [
               [...]
                "--connect",
                "port=swd reset=Crst",
                [...]
            ],
        ],

So test is configuring "reset_mode": "core", as west runner command parameter.
Then it verifies it is correctly translated into reset=Crst, which is the correct way to provide arguments to STM32_Programmer_CLI.

However, I can see that all board descriptions which provide stm32cubeprogrammer runner configuration are put as:

board_runner_args(stm32cubeprogrammer "--port=swd" "--reset=hw")

which seems indeed incorrect at first sight.

Though, I've just made some tests and it seems to be a python courtesy to accept shorten parameter names:

  • --reset-mode=hw, --reset=hw --reset-m=hw, --res=hw: all are seen correct by python and allows to generate expected -connect 'reset=HWrst' configuration
  • --resetmode generates an error

So:

  • STM32CubeProgrammer runner test looks correct to me.
  • Even if reset were used instead of reset-mode, it would not be strictly incorrect.

Did I miss something ?

@nordicjm
Copy link
Collaborator Author

nordicjm commented Jan 4, 2023

@mbolivar this seems like a bug, why is python allowing the command line arguments to be shortened?

@nordicjm
Copy link
Collaborator Author

nordicjm commented Jan 4, 2023

Seems you are right, this flag is enabled by default: https://docs.python.org/3/library/argparse.html#allow-abbrev
I will submit an issue and patch to disable it globally, the mdb test case also falls over when it's disabled.

@erwango
Copy link
Member

erwango commented Jan 5, 2023

@nordicjm Closing as this is not the bug you're looking for.

@erwango erwango closed this as completed Jan 5, 2023
nordicjm added a commit to nordicjm/zephyr that referenced this issue Jan 12, 2023
@nordicjm
west: runners: Add option to allow/prevent board reset
e87dcc4 
This adds supports for specifying if boards should be reset or not
after an image has been flashed, this is most useful for sysbuild
where multiple images are flashed and having one boot before all
are flashed causes the debug interface to be disabled or faults,
preventing correct operation of the firmware. Existing reset
priority of runners that supported it has been maintained.

This can be enabled per-board by creating a `flash_runner.yml` file
in the board directory with a list named `board_delay_reset`, e.g.
to delay an nRF5340 board from resetting whilst network core images
are being flashed, or whilst application core images (secure and
non-secure combined) are being flashed, the file contents would be
similar to:

board_delay_reset:
  - [
      nrf5340dk_nrf5340_cpuapp,
      nrf5340dk_nrf5340_cpuapp_ns,
    ]
  - [
      nrf5340dk_nrf5340_cpunet,
    ]

Also fixes zephyrproject-rtos#53487 by using the correct argument for the
stm32cubeprogrammer tests of "--reset-mode" instead of "--reset".

Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
nordicjm added a commit to nordicjm/zephyr that referenced this issue Jan 19, 2023
@nordicjm
west: runners: Add option to allow/prevent board reset
5630320 
This adds supports for specifying if boards should be reset or not
after an image has been flashed, this is most useful for sysbuild
where multiple images are flashed and having one boot before all
are flashed causes the debug interface to be disabled or faults,
preventing correct operation of the firmware. Existing reset
priority of runners that supported it has been maintained.

This can be enabled per-board by creating a `flash_runner.yml` file
in the board directory with a list named `board_delay_reset`, e.g.
to delay an nRF5340 board from resetting whilst network core images
are being flashed, or whilst application core images (secure and
non-secure combined) are being flashed, the file contents would be
similar to:

board_delay_reset:
  - [
      nrf5340dk_nrf5340_cpuapp,
      nrf5340dk_nrf5340_cpuapp_ns,
    ]
  - [
      nrf5340dk_nrf5340_cpunet,
    ]

Also fixes zephyrproject-rtos#53487 by using the correct argument for the
stm32cubeprogrammer tests of "--reset-mode" instead of "--reset".

Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
nordicjm added a commit to nordicjm/zephyr that referenced this issue Jan 19, 2023
@nordicjm
west: runners: Add option to allow/prevent board reset
cedcff0 
This adds supports for specifying if boards should be reset or not
after an image has been flashed, this is most useful for sysbuild
where multiple images are flashed and having one boot before all
are flashed causes the debug interface to be disabled or faults,
preventing correct operation of the firmware. Existing reset
priority of runners that supported it has been maintained.

This can be enabled per-board by creating a `flash_runner.yml` file
in the board directory with a list named `board_delay_reset`, e.g.
to delay an nRF5340 board from resetting whilst network core images
are being flashed, or whilst application core images (secure and
non-secure combined) are being flashed, the file contents would be
similar to:

board_delay_reset:
  - [
      nrf5340dk_nrf5340_cpuapp,
      nrf5340dk_nrf5340_cpuapp_ns,
    ]
  - [
      nrf5340dk_nrf5340_cpunet,
    ]

Also fixes zephyrproject-rtos#53487 by using the correct argument for the
stm32cubeprogrammer tests of "--reset-mode" instead of "--reset".

Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
nordicjm added a commit to nordicjm/zephyr that referenced this issue Jan 19, 2023
@nordicjm
west: runners: Add option to allow/prevent board reset
3cb2642 
This adds supports for specifying if boards should be reset or not
after an image has been flashed, this is most useful for sysbuild
where multiple images are flashed and having one boot before all
are flashed causes the debug interface to be disabled or faults,
preventing correct operation of the firmware. Existing reset
priority of runners that supported it has been maintained.

This can be enabled per-board by creating a `flash_runner.yml` file
in the board directory with a list named `board_delay_reset`, e.g.
to delay an nRF5340 board from resetting whilst network core images
are being flashed, or whilst application core images (secure and
non-secure combined) are being flashed, the file contents would be
similar to:

board_delay_reset:
  - [
      nrf5340dk_nrf5340_cpuapp,
      nrf5340dk_nrf5340_cpuapp_ns,
    ]
  - [
      nrf5340dk_nrf5340_cpunet,
    ]

Also fixes zephyrproject-rtos#53487 by using the correct argument for the
stm32cubeprogrammer tests of "--reset-mode" instead of "--reset".

Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
nordicjm added a commit to nordicjm/zephyr that referenced this issue Jan 19, 2023
@nordicjm
west: runners: Add option to allow/prevent board reset
4c30d9e 
This adds supports for specifying if boards should be reset or not
after an image has been flashed, this is most useful for sysbuild
where multiple images are flashed and having one boot before all
are flashed causes the debug interface to be disabled or faults,
preventing correct operation of the firmware. Existing reset
priority of runners that supported it has been maintained.

This can be enabled per-board by creating a `flash_runner.yml` file
in the board directory with a list named `board_delay_reset`, e.g.
to delay an nRF5340 board from resetting whilst network core images
are being flashed, or whilst application core images (secure and
non-secure combined) are being flashed, the file contents would be
similar to:

board_delay_reset:
  - [
      nrf5340dk_nrf5340_cpuapp,
      nrf5340dk_nrf5340_cpuapp_ns,
    ]
  - [
      nrf5340dk_nrf5340_cpunet,
    ]

Also fixes zephyrproject-rtos#53487 by using the correct argument for the
stm32cubeprogrammer tests of "--reset-mode" instead of "--reset".

Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
nordicjm added a commit to nordicjm/zephyr that referenced this issue Jan 19, 2023
@nordicjm
west: runners: Add option to allow/prevent board reset
2a35909 
This adds supports for specifying if boards should be reset or not
after an image has been flashed, this is most useful for sysbuild
where multiple images are flashed and having one boot before all
are flashed causes the debug interface to be disabled or faults,
preventing correct operation of the firmware. Existing reset
priority of runners that supported it has been maintained.

This can be enabled per-board by creating a `flash_runner.yml` file
in the board directory with a list named `board_delay_reset`, e.g.
to delay an nRF5340 board from resetting whilst network core images
are being flashed, or whilst application core images (secure and
non-secure combined) are being flashed, the file contents would be
similar to:

board_delay_reset:
  - [
      nrf5340dk_nrf5340_cpuapp,
      nrf5340dk_nrf5340_cpuapp_ns,
    ]
  - [
      nrf5340dk_nrf5340_cpunet,
    ]

Also fixes zephyrproject-rtos#53487 by using the correct argument for the
stm32cubeprogrammer tests of "--reset-mode" instead of "--reset".

Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
nordicjm added a commit to nordicjm/zephyr that referenced this issue Jan 19, 2023
@nordicjm
west: runners: Add option to allow/prevent board reset
c214b0f 
This adds supports for specifying if boards should be reset or not
after an image has been flashed, this is most useful for sysbuild
where multiple images are flashed and having one boot before all
are flashed causes the debug interface to be disabled or faults,
preventing correct operation of the firmware. Existing reset
priority of runners that supported it has been maintained.

This can be enabled per-board by creating a `flash_runner.yml` file
in the board directory with a list named `board_delay_reset`, e.g.
to delay an nRF5340 board from resetting whilst network core images
are being flashed, or whilst application core images (secure and
non-secure combined) are being flashed, the file contents would be
similar to:

board_delay_reset:
  - [
      nrf5340dk_nrf5340_cpuapp,
      nrf5340dk_nrf5340_cpuapp_ns,
    ]
  - [
      nrf5340dk_nrf5340_cpunet,
    ]

Also fixes zephyrproject-rtos#53487 by using the correct argument for the
stm32cubeprogrammer tests of "--reset-mode" instead of "--reset".

Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
nordicjm added a commit to nordicjm/zephyr that referenced this issue Jan 23, 2023
@nordicjm
west: runners: Add option to allow/prevent board reset
704a774 
This adds supports for specifying if boards should be reset or not
after an image has been flashed, this is most useful for sysbuild
where multiple images are flashed and having one boot before all
are flashed causes the debug interface to be disabled or faults,
preventing correct operation of the firmware. Existing reset
priority of runners that supported it has been maintained.

This can be enabled per-board by creating a `flash_runner.yml` file
in the board directory with a list named `board_delay_reset`, e.g.
to delay an nRF5340 board from resetting whilst network core images
are being flashed, or whilst application core images (secure and
non-secure combined) are being flashed, the file contents would be
similar to:

board_delay_reset:
  - [
      nrf5340dk_nrf5340_cpuapp,
      nrf5340dk_nrf5340_cpuapp_ns,
    ]
  - [
      nrf5340dk_nrf5340_cpunet,
    ]

Also fixes zephyrproject-rtos#53487 by using the correct argument for the
stm32cubeprogrammer tests of "--reset-mode" instead of "--reset".

Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nordicjm nordicjm

Labels
area: Flash area: West West utility bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32
Projects
None yet
Milestone
No milestone
2 participants
@erwango @nordicjm