Skip to content

Conversation

MrKevinWeiss
Copy link
Contributor

@MrKevinWeiss MrKevinWeiss commented Aug 9, 2021

Contribution description

Add kconfig dependency modeling for cc2538 boards based of @leandrolanzieri work.
This adds the support for the cc2538 remote based boards and radio.
It also adds some ieee802154 based kconfig tests.

Testing procedure

Check murdock is green and kconfig tests are run for the remote-revb

Issues/PRs references

@MrKevinWeiss MrKevinWeiss added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 9, 2021
@MrKevinWeiss MrKevinWeiss self-assigned this Aug 9, 2021
@github-actions github-actions bot added Area: boards Area: Board ports Area: build system Area: Build system Area: CI Area: Continuous Integration of RIOT components Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: network Area: Networking Area: sys Area: System Area: tests Area: tests and testing framework Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Aug 9, 2021
@MrKevinWeiss
Copy link
Contributor Author

I just want to check the CI output first to see what the state is. It looks like there is some radio work done but maybe that would be better to add in another PR (especially since I think it would also be testing the boards with radios such as the samr)

@MrKevinWeiss MrKevinWeiss changed the title Pr/kconfig/cc2538 cpu/cc2538: Add Kconfig support Aug 10, 2021
@MrKevinWeiss MrKevinWeiss removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Aug 10, 2021
@MrKevinWeiss
Copy link
Contributor Author

Some discussion was had about how saul_default should select things and we reached some conclusion that generally the HAVE_x in the board definition should be responsible for selecting the x part if saul_default is enabled but that will be implemented in a different PR.

@RIOT-OS RIOT-OS deleted a comment from riot-ci Aug 11, 2021
@MrKevinWeiss
Copy link
Contributor Author

finally got it working... ignore the bot

@MrKevinWeiss
Copy link
Contributor Author

Lets see what murdock says.

@MrKevinWeiss
Copy link
Contributor Author

There we go, so I just removed the submac test as we are not adding netdev for this PR.

@RIOT-OS RIOT-OS deleted a comment from riot-hil-bot Sep 3, 2021
@RIOT-OS RIOT-OS deleted a comment from riot-hil-bot Sep 6, 2021
@RIOT-OS RIOT-OS deleted a comment from riot-hil-bot Sep 6, 2021
@MrKevinWeiss
Copy link
Contributor Author

Rebased and fixed the DEVELHELP export (and added some documentation on the variable).

ping @fjmolinas

To test changes

cd tests/ieee802154_hal
export BOARD=remote-revb
make clean && DEVELHELP=0 TEST_KCONFIG=1 make info-modules | grep mpu
#Should be empty
make clean && TEST_KCONFIG=1 make info-modules | grep mpu
#Should have mpu_stack_guard

You can repeat without TEST_KCONFIG to verify it is the same with make.

@fjmolinas
Copy link
Contributor

Rebased and fixed the DEVELHELP export (and added some documentation on the variable).

ping @fjmolinas

To test changes

cd tests/ieee802154_hal
export BOARD=remote-revb
make clean && DEVELHELP=0 TEST_KCONFIG=1 make info-modules | grep mpu
#Should be empty
make clean && TEST_KCONFIG=1 make info-modules | grep mpu
#Should have mpu_stack_guard

You can repeat without TEST_KCONFIG to verify it is the same with make.

Look good to me, IMO you can squash, any remaining concerns @leandrolanzieri?

@leandrolanzieri
Copy link
Contributor

any remaining concerns @leandrolanzieri?

None, it looks good to me!

Copy link
Contributor

@leandrolanzieri leandrolanzieri left a comment

Choose a reason for hiding this comment

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

ACK!

@leandrolanzieri leandrolanzieri added this to the Release 2021.10 milestone Sep 8, 2021
@leandrolanzieri
Copy link
Contributor

There seems to be an unrelated issue on the CI

@fjmolinas fjmolinas added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 8, 2021
@fjmolinas
Copy link
Contributor

There seems to be an unrelated issue on the CI

Seems like some kind of lack of space issue.. re-triggered

@MrKevinWeiss MrKevinWeiss merged commit 4c6dc97 into RIOT-OS:master Sep 9, 2021
@MrKevinWeiss MrKevinWeiss deleted the pr/kconfig/cc2538 branch September 9, 2021 06:21
@MrKevinWeiss
Copy link
Contributor Author

Thanks for the review and handholding :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: build system Area: Build system Area: CI Area: Continuous Integration of RIOT components Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: network Area: Networking Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants