Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Partition Manager external flash mcuboot/thingy53 #9169
Partition Manager external flash mcuboot/thingy53 #9169
Changes from all commits
b885c2e
cceeadd
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Does it need to be copied to other Matter samples as well? What has changed recently given that it wasn't necessary in the past?
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.
It probably should be done in a lot of places. I did it here hoping to fix the CHIP door lock tests that were failing, and it seems to have worked.
Basically, it needs to be done if using external flash as one of the slots for MCUboot.
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.
OK, but it used to not be needed somehow. Is it a regression then?
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.
It's basically been lucky. Except when the app is non-secure, then it has silently failed. I don't know the details about how it fails. It seems the only test that was failing was the door lock one, so maybe we can merge this, and I can open a PR that changes more samples. @tejlmand could you comment? How important is it to add the chosen to all the samples that use ext_flash and mcuboot?
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.
My point is that if this test has failed because of your change then probably other samples will fail on nightly as well. In the integration test suite we may not cover all samples. So it's good to understand which apps must be updated and update them all.
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.
@tejlmand or should I instead create a
pm_ext_flash.overlay
and do: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.
@Damian-Nordic @tejlmand I added overlay files to a number of samples now
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.
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.
@oyvindronningstad Don't we need also to align
matter/window_covering
andapplications/matter_weather_station
?