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.

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

doc: add suit SMP readme #15287

Merged
merged 1 commit into from
Jun 5, 2024
Merged

Conversation

kittydepa
Copy link

Add README page for SUIT SMP sample

@kittydepa kittydepa added the DNM label May 14, 2024
@github-actions github-actions bot added doc-required PR must not be merged without tech writer approval. changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels May 14, 2024
@kittydepa kittydepa requested a review from greg-fer as a code owner May 14, 2024 07:33
@NordicBuilder
Copy link
Contributor

NordicBuilder commented May 14, 2024

Test specification

CI/Jenkins/NRF

  • Skipped

CI/Jenkins/integration

  • Skipped

Note: This message is automatically posted and updated by the CI

samples/suit/smp_transfer/README.rst Show resolved Hide resolved
samples/suit/smp_transfer/README.rst Outdated Show resolved Hide resolved
samples/suit/smp_transfer/README.rst Show resolved Hide resolved
samples/suit/smp_transfer/README.rst Outdated Show resolved Hide resolved
samples/suit/smp_transfer/README.rst Outdated Show resolved Hide resolved
samples/suit/smp_transfer/README.rst Outdated Show resolved Hide resolved
samples/suit/smp_transfer/README.rst Outdated Show resolved Hide resolved
* :file:`hci_rpmsg_subimage.suit` - This file is the envelop for the Radio Core, containing embedded radio core firmware as an integrated payload.

For more information about files generated as output of the build process, see :ref:`app_build_output_files`.
For more information on the contents of the build directory, see :ref:`zephyr:build-directory-contents` in the Zephyr documentation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is a specific page that show the file structure generated by sysbuild build, it will be worth to use it instead (there are some changes, i.e. application artifacts are under a dedicated subfolder).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! Will look into this and get back to this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The link you provided looks different than the one you implemented, @kittydepa , so I'm not sure if this line has been updated. Anyway, I'm not sure if providing a link to output files in the NCS with the detailed tables first, and then linking to a generic section in Zephyr with way less detailed list makes sense.

samples/suit/smp_transfer/README.rst Outdated Show resolved Hide resolved
samples/suit/smp_transfer/README.rst Outdated Show resolved Hide resolved
@kittydepa kittydepa requested a review from tomchy May 14, 2024 11:39
@kittydepa kittydepa force-pushed the doc_add_suit_smp_readme branch 2 times, most recently from 3a806de to 9c229fa Compare May 14, 2024 11:46
@kittydepa kittydepa force-pushed the doc_add_suit_smp_readme branch 3 times, most recently from de30f28 to e8757d6 Compare May 15, 2024 06:43
@NordicBuilder
Copy link
Contributor

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

@kittydepa kittydepa requested a review from richabp May 15, 2024 07:24
@kittydepa kittydepa added this to the 2.7.0 milestone May 15, 2024
@kittydepa kittydepa force-pushed the doc_add_suit_smp_readme branch 2 times, most recently from ff2b1f5 to 2d97476 Compare May 16, 2024 08:06
@kittydepa kittydepa removed changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. DNM labels May 16, 2024

|config|

The default configuration uses UART with Version 1.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version -> sequence number ?

Copy link
Author

@kittydepa kittydepa May 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomchy Would it be okay to rewrite to include both? Something like:

Suggested change
The default configuration uses UART with Version 1.
The default configuration uses UART with sequence number 1 (shown as Version 1 in the nRF Connect Device Manager app).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, as long as you talk about nRF Connect Device Manager app it is fine 👍

|config|

The default configuration uses UART with Version 1.
To change the version of the application, configure the :ref:`CONFIG_SUIT_ENVELOPE_SEQUENCE_NUM` Kconfig option.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

version -> sequence number ?

To change the version of the application, configure the :ref:`CONFIG_SUIT_ENVELOPE_SEQUENCE_NUM` Kconfig option.
This also changes the number of blinks on **LED 1** and sets the :ref:`sequence number <ug_suit_dfu_suit_manifest_elements>` of the SUIT :ref:`envelope <ug_suit_dfu_suit_concepts>`’s manifest.

To use this configuration, include the ``-DCONFIG_SUIT_ENVELOPE_SEQUENCE_NUM=x`` option with the build command, where x is the version number.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once again changed CONFIG_SUIT_ENVELOPE_SEQUENCE_NUM to a sysbuild KConfig: SB_CONFIG_SUIT_ENVELOPE_SEQUENCE_NUM. That way it is a common value for all manifests that are generated by the build system.


west build -p -b nrf54h20dk/nrf54h20/cpuapp --sysbuild -- -DCONFIG_SUIT_ENVELOPE_SEQUENCE_NUM=2

If you do not specify this configuration, the sample is built with Version 1.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version -> sequence number


.. _CONFIG_SUIT_ENVELOPE_SEQUENCE_NUM:

CONFIG_SUIT_ENVELOPE_SEQUENCE_NUM - Configuration for the application version
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rephrase all references to the application version.

* :file:`root.suit` - This file is the most important envelope, containing the embedded :file:`application.suit` and :file:`radio.suit` files as integrated dependencies.
These files are used to execute DFU.
* :file:`application.suit` - This file is the envelope for the Application Core, containing embedded Application Core firmware as an integrated payload.
* :file:`radio.suit` - This file is the envelop for the Radio Core, containing embedded radio core firmware as an integrated payload.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: envelop -> envelope


If you want to further configure your sample, see :ref:`configure_application` for additional information.

After running the ``west build`` command, the output build files can be found in the :file:`build/zephyr` directory.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

build/zephyr -> build/DFU


.. code-block:: console

west build -p -b nrf54h20dk/nrf54h20/cpuapp -d C:\ncs-lcs\west_working_dir\build\
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing: --sysbuild


.. code-block:: console

west build -p -b nrf54h20dk/nrf54h20/cpuapp -d C:/ncs-lcs/work-dir -- -DFILE_SUFFIX=bt -DCONFIG_SUIT_ENVELOPE_SEQUENCE_NUM=2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing: --sysbuild

Comment on lines 7 to 11
:maxdepth: 1
:caption: Subpages
:glob:

../../../samples/suit/*/README
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this samples/suit.rst is basically being emptied out, shouldn't it be orphaned too?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought so too, but just got latest info from Francesco that it's not needed/okay to be un-orphaned now.

Comment on lines 224 to 227
Building and programming using |nRFVSC|
=======================================

To build the sample using |VSC|, follow the steps listed on the `How to build an application`_ page in the |nRFVSC| documentation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think nRFVSC being the recommended IDE, it should be listed first, before the command line steps.
Also, have this sample been tested with nRF VSC extension? @tomchy ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has been verified once in sdk-nrf-next and the next round is planned to be a part of NCSDK-24867.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, will remove for now but have added to an existing ticket to watch for this/update if needed


.. group-tab:: Over Bluetooth Low Energy

1. **Update the envelope sequence number:**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't bold out the step descriptions, as per style guide :(
Also, while I think this bold should not be here, it is also not applied consistently to all steps.

Comment on lines 240 to 247
Build the sample with an updated sequence number:

.. code-block:: console

west build -p -b nrf54h20dk/nrf54h20/cpuapp --sysbuild -- -DFILE_SUFFIX=bt -DCONFIG_SUIT_ENVELOPE_SEQUENCE_NUM=2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the pickle: normally, for all samples, Testing is about pressing some buttons or verifying that the programmed firmware works. Here, in this sample, Testing includes building steps.

Also note that only command line building steps are provided.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still looking into this, with @richabp

.. figure:: /images/suit_smp_select_image_read.png
:alt: Select READ from Images

Observe "Version: 1" printed in the **Images** section of the mobile app.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Global comment for multiple occurrences)
Starting with a verb makes this technically a separate step, an action to be taken.
A way to go around this and keep one step is to rephrase this line so that it reads like a result of the action.
Something like ""Version: 1" is listed in the Images section of the mobile app." or "The Images section
now mentions "Version: 1".

Comment on lines 198 to 216
After running the ``west build`` command, the output build files can be found in the :file:`build/dfur` directory.
The following build artifacts are found with both firmware binaries embedded as integrated payloads:

* :file:`root.suit` - This file is the most important envelope, containing embedded :file:`app.suit` as an integrated dependency.
This file is used to execute DFU.
* :file:`app.suit` - This file is the envelope for the Application Core, containing embedded Application Core firmware as an integrated payload.

For more information about files generated as output of the build process, see :ref:`app_build_output_files`.
For more information on the contents of the build directory, see :ref:`zephyr:build-directory-contents` in the Zephyr documentation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

Comment on lines 210 to 228
#. Program the sample to the kit using the following command:

.. code-block:: console

west flash

.. note::

If you are compiling in Windows and the build is unsuccessful due to the maximum path length limitation, use the following command:

.. code-block:: console

west flash -d C:/ncs-lcs/work-dir
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

Comment on lines 104 to 105
Manifest template files
=======================
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The title suggests to me I would find here a list of the template files. Not finding the detailed list and having the note here that mentions the radio domain manifest template makes me really confused. Only later, when I read about building, can I really understand what this note means. (I guess you mean radio.suit here?) For this reason, I think the order of presenting information here is not optimal.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good point - I will take a look and try to reorder.
I have also just realised that this note may not be true anymore, and then can be removed (will check with SMEs). But will still try to adjust the order

Comment on lines 33 to 34
MCUmgr is a tool that can be used to upload SUIT envelopes through the SMP protocol.
For more information on MCUmgr installation, see :ref:`zephyr:mcu_mgr`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that one also needs to (or can) use the samples/common MCUmgr component if I want to use MCUmgr?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not clear to me what do you mean by MCUmgr component, but going into the details we provide an alternative implementation of the image command group as well as a dedicated, custom suit command group for SMP protocol.

That said, yes, you can extend the sample with other SMP commands with the exception of the image group, since the code should not include two implementations of the same group (already blocked through Kconfig dependencies).

Any tool that supports SMP protocol over UART or BLE can be used with the sample.

Copy link
Author

@kittydepa kittydepa May 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomchy I believe Greg maybe referring to the CONFIG_NCS_SAMPLE_MCUMGR_BT_OTA_DFU Kconfig option found in samples/common (See here for description. It is currently recommended for BLE FOTA updates on nRF52 Series and nRF5340 DK).

My guess is no, it is not required and/or it has not been tested for SUIT specifically yet?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'm referring to CONFIG_NCS_SAMPLE_MCUMGR_BT_OTA_DFU. When we discussed CONFIG_NCS_SAMPLE_MCUMGR_BT_OTA_DFU in the call, Kamil Piszczek said it can be used with whatever sample that uses Bluetooth LE, so I'd like to make sure there are no collisions between that Kconfig option ("MCUmgr component") and this implementation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It cannot be used ATM, because it depends on BOOTLOADER_MCUBOOT which is not compatible with nRF54Hxx devices.
I have not heard of this Kconfig before and I do not know if it should be supported on nRF54H (so it would depend on the SUIT and our implementation of SMP image group).

Overview
********

The sample uses one of the following to perform an update on the nRF54H20 SoC, featuring Nordic Semiconductor’s implementation of the SUIT procedure:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The sample uses one of the following to perform an update on the nRF54H20 SoC, featuring Nordic Semiconductor’s implementation of the SUIT procedure:
The sample uses one of the following methods to perform a firmware update on the nRF54H20 SoC, featuring Nordic Semiconductor’s implementation of the SUIT procedure:

@kittydepa
Copy link
Author

Just pushed what I have completed so far, still working to address remaining suggestions.

@greg-fer
Copy link
Contributor

Just pushed what I have completed so far, still working to address remaining suggestions.

OK, thanks @kittydepa . Poke me when you address all and want me to have a look.

@kittydepa kittydepa force-pushed the doc_add_suit_smp_readme branch 3 times, most recently from 5e4bae7 to 73f4965 Compare May 24, 2024 12:00
@kittydepa kittydepa requested review from tomchy and greg-fer May 24, 2024 12:01
@kittydepa kittydepa added DNM and removed changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels May 24, 2024
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label May 24, 2024
@kittydepa kittydepa force-pushed the doc_add_suit_smp_readme branch 2 times, most recently from b36bd5b to fdd2fe2 Compare May 24, 2024 13:43
:local:
:depth: 2

The sample demonstrates how to update and boot the nRF54H20 System-on-Chip (SoC) using the Software Update for Internet of Things (SUIT) procedure on the application and radio cores of the SoC.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link Software Update for Internet of Things (SUIT) to a generic SUIT intro page (in the NCS docs or external) or to a term in the glossary (which we also need anyway, actually).

Comment on lines +58 to +56
LED 1:
The number of blinks indicates the application SUIT envelope sequence number.
The default is set to ``1`` to blink once, indicating "Version 1".
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for SME response.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can apply here the rules Bartek posted after the meeting with Paal.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first one. The number changes from DK to DK. I have no idea what will be written on the final one.

To change the sequence number of the application, configure the :ref:`CONFIG_SUIT_ENVELOPE_SEQUENCE_NUM` Kconfig option.
This also changes the number of blinks on **LED 1** and sets the :ref:`sequence number <ug_suit_dfu_suit_manifest_elements>` of the SUIT :ref:`envelope <ug_suit_dfu_suit_concepts>`’s manifest.

To use this configuration, include the ``SB_CONFIG_SUIT_ENVELOPE_SEQUENCE_NUM=x`` option with the build command, where x is the version number.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
To use this configuration, include the ``SB_CONFIG_SUIT_ENVELOPE_SEQUENCE_NUM=x`` option with the build command, where x is the version number.
To use this configuration, build the sample with sysbuild and set the ``SB_CONFIG_SUIT_ENVELOPE_SEQUENCE_NUM`` Sysbuild Kconfig option to `x`, where `x` is the version number.


west build -p -b nrf54h20dk/nrf54h20/cpuapp --sysbuild -- -SB_CONFIG_SUIT_ENVELOPE_SEQUENCE_NUM=2

If you do not specify this configuration, the sample is built with sequence number 1 (shown as Version 1 in the nRF Device Manager app.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If you do not specify this configuration, the sample is built with sequence number 1 (shown as Version 1 in the nRF Device Manager app.
If you do not specify this configuration, the sample is built with sequence number 1 (shown as Version 1 in the nRF Device Manager app).

All required manifest files (used to later create SUIT envelopes) are automatically created after the sample is built for the first time, and are the following:

* The root manifest - :file:`root_hierarchical_envelope.yaml.jinja2`

* The application domain manifest - :file:`app_envelope.yaml.jinja2`

* The radio domain manifest - :file:`rad_envelope.yaml.jinja2`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can now link to output build files here too! :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah! Will add.

Comment on lines 160 to 168
The output build files can be found in the :file:`build/DFU` directory.
The following build artifacts are found with both firmware binaries embedded as integrated payloads:

* :file:`root.suit` - This file is the most important envelope, containing the embedded :file:`application.suit` and :file:`radio.suit` files as integrated dependencies.
These files are used to execute DFU.
* :file:`application.suit` - This file is the envelope for the application core, containing embedded application core firmware as an integrated payload.
* :file:`radio.suit` - This file is the envelope for the radio core, containing embedded radio core firmware as an integrated payload.

For more information about files generated as output of the build process, see :ref:`app_build_output_files`, specifically :ref:`app_build_output_files_suit_dfu`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just like this?

Suggested change
The output build files can be found in the :file:`build/DFU` directory.
The following build artifacts are found with both firmware binaries embedded as integrated payloads:
* :file:`root.suit` - This file is the most important envelope, containing the embedded :file:`application.suit` and :file:`radio.suit` files as integrated dependencies.
These files are used to execute DFU.
* :file:`application.suit` - This file is the envelope for the application core, containing embedded application core firmware as an integrated payload.
* :file:`radio.suit` - This file is the envelope for the radio core, containing embedded radio core firmware as an integrated payload.
For more information about files generated as output of the build process, see :ref:`app_build_output_files`, specifically :ref:`app_build_output_files_suit_dfu`.
The output build files can be found in the :file:`build/DFU` directory, including the :ref:`app_build_output_files_suit_dfu`.

Comment on lines 226 to 233
After running the ``west build`` command, the output build files can be found in the :file:`build/dfu` directory.
The following build artifacts are found with both firmware binaries embedded as integrated payloads:

* :file:`root.suit` - This file is the most important envelope, containing embedded :file:`app.suit` as an integrated dependency.
This file is used to execute DFU.
* :file:`application.suit` - This file is the envelope for the application core, containing embedded application core firmware as an integrated payload.

For more information about files generated as output of the build process, see :ref:`app_build_output_files`, specifically :ref:`app_build_output_files_suit_dfu`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above?

1. Upload the signed envelope onto your mobile phone:

a. Open the nRF Device Manager app on your mobile phone.
#. Select the device **SUIT SMP Sample**. You should see the following:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#. Select the device **SUIT SMP Sample**. You should see the following:
#. Select the device **SUIT SMP Sample**.
You should see the following:

@kittydepa kittydepa requested a review from greg-fer June 3, 2024 09:43
@kittydepa kittydepa removed DNM changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Jun 4, 2024
Copy link
Contributor

@greg-fer greg-fer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, though with additional comments. Mostly suggestions.

@@ -0,0 +1,382 @@
:orphan:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this won't be applied in the current structure, because the :orphaned: SUIT samples are made visible by the toctree inclusion on this page: https://developer.nordicsemi.com/nRF_Connect_SDK/doc/latest/nrf/samples/suit.html

Both samples visible there are orphaned.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed :orphan: label


.. _nrf54h_suit_sample:

SUIT DFU: Firmware update on the nRF54H20 SoC
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
SUIT DFU: Firmware update on the nRF54H20 SoC
SUIT: Device firmware update on the nRF54H20 SoC

To me, SUIT itself is about DFU, DFU next to it is also about DFU, and then we have "firmware update" right after it. This is a lot of firmware updates in one title.

Comment on lines 32 to 34
For a SUIT update over UART, you need to install MCUmgr.
MCUmgr is a tool that can be used to upload SUIT envelopes through the SMP protocol.
For more information on MCUmgr installation, see :ref:`zephyr:mcu_mgr`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
For a SUIT update over UART, you need to install MCUmgr.
MCUmgr is a tool that can be used to upload SUIT envelopes through the SMP protocol.
For more information on MCUmgr installation, see :ref:`zephyr:mcu_mgr`.
For a SUIT update over UART, you need to install :ref:`zephyr:mcu_mgr`, a tool that can be used to upload SUIT envelopes through the SMP protocol.

Shorter suggestion, nothing wrong with the original.

Comment on lines +58 to +56
LED 1:
The number of blinks indicates the application SUIT envelope sequence number.
The default is set to ``1`` to blink once, indicating "Version 1".
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can apply here the rules Bartek posted after the meeting with Paal.


The default configuration uses UART with sequence number 1 (shown as Version 1 in the nRF Device Manager app).
To change the sequence number of the application, configure the :ref:`CONFIG_SUIT_ENVELOPE_SEQUENCE_NUM` Kconfig option.
This also changes the number of blinks on **LED 1** and sets the :ref:`sequence number <ug_suit_dfu_suit_manifest_elements>` of the SUIT :ref:`envelope <ug_suit_dfu_suit_concepts>`’s manifest.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This also changes the number of blinks on **LED 1** and sets the :ref:`sequence number <ug_suit_dfu_suit_manifest_elements>` of the SUIT :ref:`envelope <ug_suit_dfu_suit_concepts>`s manifest.
This also changes the number of blinks on **LED 1** and sets the :ref:`sequence number <ug_suit_dfu_suit_manifest_elements>` of the :ref:`SUIT envelope <ug_suit_dfu_suit_concepts>`'s manifest.

To change the sequence number of the application, configure the :ref:`CONFIG_SUIT_ENVELOPE_SEQUENCE_NUM` Kconfig option.
This also changes the number of blinks on **LED 1** and sets the :ref:`sequence number <ug_suit_dfu_suit_manifest_elements>` of the SUIT :ref:`envelope <ug_suit_dfu_suit_concepts>`’s manifest.

To use this configuration, build the sample with sysbuild and set the ``SB_CONFIG_SUIT_ENVELOPE_SEQUENCE_NUM`` Sysbuild Kconfig option to ``x``, where ``x`` is the version number.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
To use this configuration, build the sample with sysbuild and set the ``SB_CONFIG_SUIT_ENVELOPE_SEQUENCE_NUM`` Sysbuild Kconfig option to ``x``, where ``x`` is the version number.
To use this configuration, build the sample with :ref:`configuration_system_overview_sysbuild` and set the ``SB_CONFIG_SUIT_ENVELOPE_SEQUENCE_NUM`` Sysbuild Kconfig option to ``x``, where ``x`` is the version number.

Comment on lines 99 to 101
* To modify the DFU partition, modify the values for ``dfu_partition`` defined in :file:`samples/suit/smp_transfer/boards/nrf54h20dk_nrf54h20_cpuapp.overlay`.

* ``dfu_partition`` is where the update candidate is stored before the update process begins.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* To modify the DFU partition, modify the values for ``dfu_partition`` defined in :file:`samples/suit/smp_transfer/boards/nrf54h20dk_nrf54h20_cpuapp.overlay`.
* ``dfu_partition`` is where the update candidate is stored before the update process begins.
* To modify the DFU partition, modify the values for ``dfu_partition`` defined in :file:`samples/suit/smp_transfer/boards/nrf54h20dk_nrf54h20_cpuapp.overlay`.
This partition is where the update candidate is stored before the update process begins.

This is a nit.
If the intention is to highlight the importance of this partition, then maybe you can add this entry between lines 94 and 95, in a list that describes the purpose of both this partition and cpuapp_slot0_partition? Just a suggestion.

Comment on lines +244 to +247
.. note::

|application_sample_long_path_windows|

In this case, you may need to run the following instead:

.. code-block:: console

west build -p -b nrf54h20dk/nrf54h20/cpuapp -d C:/ncs-lcs/work-dir --sysbuild -- -SB_CONFIG_SUIT_ENVELOPE_SEQUENCE_NUM=2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, would it be possible to post this note at the beginning of the procedure once, with some generic information, instead of repeating it 5 times within the same procedure? I know the command is different each time, but also what is critical for this information -- -d <path> -- stays the same.


Check and configure the following configuration option for the sample:

.. _CONFIG_SUIT_ENVELOPE_SEQUENCE_NUM:
Copy link
Contributor

@tomchy tomchy Jun 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use a dedicated format for sysbuild Kconfigs, please apply it here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied

+------------------------------------------------+------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| File | Description |
+================================================+========================================================================================================================================================================+
| :file:`root_hierarchical_envelope.yaml.jinja2` | SUIT Manifest templates automatically placed in the sample directory after the first build of the ``nrf54h_suit_sample`` sample. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This time build system produced root_with_binary_nordic_top.yaml.jinja2 instead.
Feel free to update or leave it as is. I think we will make one more round later to double check all of the names, paths etc.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, will change it, and make a note for all of us to review this table. @richabp Just FYI

@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jun 4, 2024
@kittydepa kittydepa removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jun 4, 2024
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jun 4, 2024
Add README page for SUIT SMP sample

Signed-off-by: Katherine Depa <[email protected]>
@rlubos rlubos merged commit e553c49 into nrfconnect:main Jun 5, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. doc-required PR must not be merged without tech writer approval.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants