-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add a sample that demonstrates use of UICR.SECONDARY.TRIGGER #25002
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
Conversation
The following west manifest projects have changed revision in this Pull Request:
✅ All manifest checks OK Note: This message is automatically posted and updated by the Manifest GitHub Action. |
CI InformationTo view the history of this post, clich the 'edited' button above Inputs:Sources:sdk-nrf: PR head: 03d763090f71aef621b79bfd6d4364a6199026af more detailssdk-nrf:
zephyr:
Github labels
List of changed files detected by CI (20)
Outputs:ToolchainVersion: 46667c6630 Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
6af3a35
to
d110575
Compare
You can find the documentation preview for this PR here. Preview links for modified nRF Connect SDK documents: https://ncsdoc.z6.web.core.windows.net/PR-25002/nrf/app_dev/device_guides/nrf54h/ug_nrf54h20_ironside.html |
samples.secondary_boot.nrf54h20dk: | ||
tags: | ||
- sysbuild | ||
- ci_samples_secondary_boot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add full path to tag name e.g ci_samples_ironside_se_secondary_boot
or just ci_samples_ironside_se
.
To actually work, this tag needs to be defined here: https://github.com/nrfconnect/sdk-nrf/blob/main/scripts/ci/tags.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed the tag
samples.secondary_boot_trigger_lockup.nrf54h20dk: | ||
tags: | ||
- sysbuild | ||
- ci_samples_secondary_boot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add full path to tag name e.g ci_samples_ironside_se_secondary_boot
or just ci_samples_ironside_se
.
To actually work, this tag needs to be defined here: https://github.com/nrfconnect/sdk-nrf/blob/main/scripts/ci/tags.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed the tag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it is actually needed (CI uses it to determine scope, without it, all PRs will build those samples which does not make sense)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I was not aware, thank you for explaining.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
These samples are closely integrated with some files in
~/ncs/zephyr/soc/nordic/common/uicr/
Is there any way to express with the tag system that when we patch these files that this tag should be run?
If not, could I add
~/ncs/nrf/west.yml
to the ci_samples_sdfw tag perhaps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need simply add those paths into depencies for this tag - ci_samples_sdfw - ie in this list https://github.com/nrfconnect/sdk-nrf/blob/main/scripts/ci/tags.yaml#L1514
It supports whole ncs tree, so you can add directly path to zephyr.
Please also add valid path for those samples into dependencies!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow!
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Please also add valid path for those samples into dependencies!"
Sorry, not sure if I understand you.
That's what I'm doing here right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, exactly this, thanks.
@cursor review |
Skipping Bugbot: Bugbot is disabled for this repository |
c5b1c16
to
1b80a2f
Compare
1b80a2f
to
88e5d62
Compare
65c95d6
to
2734f11
Compare
2734f11
to
86f4529
Compare
update zephyr Signed-off-by: Sebastian Bøe <[email protected]>
run ci_samples_sdfw when nrf/samples/ironside_se is changed. Also get rid of nrf/samples/sdfw/ as it doesn't exist anymore. Signed-off-by: Sebastian Bøe <[email protected]>
Minor doc/yaml fixes Signed-off-by: Sebastian Bøe <[email protected]>
Add a sample that demonstrates use of UICR.SECONDARY.TRIGGER. Signed-off-by: Sebastian Bøe <[email protected]>
86f4529
to
03d7630
Compare
@PerMac @katgiadla @nordic-piks @nrfconnect/ncs-co-build-system @nrfconnect/ncs-test-leads PTAL, apparently GH hides missing reviews in case there's a conflict in the PR 🤷 |
Ah, I thought I had the reviews I needed. |
harness_config: | ||
type: multi_line | ||
regex: | ||
- "=== Primary Image: Demonstrating APPLICATIONLOCKUP trigger ===" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it needed specific order of the lines?
Add a sample that demonstrates use of UICR.SECONDARY.TRIGGER