pkg/semtech-loramac: model in Kconfig by aabadie · Pull Request #18005 · RIOT-OS/RIOT

aabadie · 2022-04-25T16:35:44Z

Contribution description

As the title says.

To fully match the Make dependency resolution, I had to modify the sx127x Kconfig file by removing the usage of choices. Maybe this is wrong but I don't see how to properly include MODULE_SX1276 to the build with Kconfig. Looking at tests/driver_sx127x it seems to be possible by pulling in both MODULE_SX1276 and MODULE_SX127X. That seems bogus and counter intuitive to me as I would expect the sx127x moduie to be pulled in automatically like it's done by this PR.

Note that this PR won't pass Murdock because of a mismatch resolution of the random module dependencies (again a choice is in the loop).

Testing procedure

Green Murdock

Issues/PRs references

#16875

leandrolanzieri · 2022-04-26T07:05:02Z

drivers/sx127x/Kconfig

-
 config MODULE_SX1272
    bool "SX1272"
+    select MODULE_SX127X


The usage of choices for the variant was a design decision for all drivers. We know that it requires to select both the driver module and variant, but that is only needed when the variant is to be forced. In case the board has the device connected to it, the HAVE_* symbol would be selected, indicating a preference for the choice.

We have considered the approach you have here in the past (actually we started modelling like that), but we realized that it would not allow us to use modules that enable defaults (MODULE_NETDEV_DEFAULT in this case). Now, when such default is enabled, the driver module would be enabled (if the correct feature is present) and the correct variant would be the default choice option. This can't be done when the entry-point is the choice. There is some info in this PR https://github.com/RIOT-OS/RIOT/pull/17556/files#diff-2132175f34702f4d76287f4df5c5f5db02d097c2778e5ac47da4c88ef8f13acdR517.

From the board with hardwired radio perspective that could make sense. From the end user that plugged the radio on its board, I think it doesn't make sense since he won't have the HAVE_ symbol defined for his board (thinking of a nucleo64 where one plugs an mbed lora shield), resulting in the user having to enable both sx127x and sx1276 (or sx1272 depending on the shield he is using).

Offline @fjmolinas told me that the choice was here to prevent both sx1272 and sx1276 to be enabled at the same time. But in make one could do that already. I would rather stick to the same behavior as make as a first step and after think of solutions for this problem later.

drivers/sx127x/Kconfig

MrKevinWeiss

It would be good if we keep the same design here.

I know that it is a bit verbose to require both the module and the variant explicitly since the variant would require the module.

This has been played with for quite some time now and it the best solution given the constraints we are dealing with.

I strongly push the HAVE_* because I think having a verbose (and machine readable) description of the board is a good thing.

We can open up the conversation and look for a better way.

aabadie · 2022-04-26T12:45:45Z

I strongly push the HAVE_* because I think having a verbose (and machine readable) description of the board is a good thing.

I have no objection with the HAVE_ and I even find this concept quite nice. If you see my latest change, MODULE_SX1276 symbol can be pull-in automatically by HAVE_SX1276 if MODULE_NETDEV_DEFAULT is selected.
This is what looks the most natural to me and this allows the automatic inclusion of MODULE_SX127X from the specific driver module.

aabadie · 2022-04-27T09:54:53Z

I removed the Kconfig changes related to sx127x driver and added the missing symbols in loramac applications config. Locally the dependency resolution is the same as with Make so I have good hopes that it will pass Murdock now.

MrKevinWeiss

I don't know if you want to note this somewhere but after the migration is complete we can probably remove most of the CONFIG_MODULE_SX1276=y and just default be that but if boards have something different then it should use it.
This is only be we need to match the make system and currently it is not implemented.

MrKevinWeiss · 2022-04-28T07:49:28Z

examples/lorawan/Kconfig

+    bool
+    default y
+    depends on TEST_KCONFIG
+    select MODULE_PERIPH_RTC if HAS_PERIPH_RTC


Maybe this could help

Suggested change

select MODULE_PERIPH_RTC if HAS_PERIPH_RTC

select MODULE_PERIPH_RTC if HAS_PERIPH_RTC && !HAVE_SHARED_PERIPH_RTT_PERIPH_RTC

aabadie · 2023-05-22T08:53:20Z

Last one failing: https://ci.riot-os.org/results/dfe74ed322d2448e82e77af2aa8cf399/output/builds/tests/sys/fido2_ctap/adafruit-itsybitsy-m4:gnu.txt

MrKevinWeiss · 2023-05-22T10:24:39Z

Ahh no fail fast, good... if this really is the last one then we can go. If there are more than maybe there needs to be a better way.

MrKevinWeiss · 2023-05-22T10:37:03Z

~~Hmm it seems an issue with bors merging multiple PRs... I wonder if we can just get it isolated...~~ ugg, just getting used to the new dir structure.

MrKevinWeiss · 2023-05-22T11:55:24Z

I guess it is because the first iteration of make has ztimer_msec and nothing else, which brings in the rtt. The later iterations bring in xtimer which brings in usec which brings in periph_timer. Since the first round cannot know periph timer will be used we are forced to bring in both.

I wonder what the ideal behaviour is, use both backeds (since the msec will be able to sleep) or stop with this, if I am using a backend for something else then just use that one (which saves space but prevents sleep).

aabadie · 2023-05-23T08:04:46Z

Murdock is finally green here!

MrKevinWeiss

ACK, lets see what a larger test will say but I think we need to move on this one. This does change make to have a more consistent ztimer backend selection... It might have some effect for those who assumed power savings over code size.

MrKevinWeiss · 2023-05-24T08:15:30Z

bors merge

18005: pkg/semtech-loramac: model in Kconfig r=MrKevinWeiss a=aabadie Co-authored-by: Alexandre Abadie <[email protected]>

aabadie · 2023-05-24T09:06:44Z

bors cancel

(trying to fill a merge 🚋 )

bors · 2023-05-24T09:06:46Z

Canceled.

aabadie · 2023-05-24T09:29:32Z

bors merge

18005: pkg/semtech-loramac: model in Kconfig r=aabadie a=aabadie 19660: cpu/rpx0xx: Fix kconfig model r=aabadie a=MrKevinWeiss ### Contribution description Broken master due to incorrect model of the periph_pio in kconfig. ### Testing procedure Green murdock (now that the board is added to the list) ### Issues/PRs references Look at the master CI... Co-authored-by: Alexandre Abadie <[email protected]> Co-authored-by: MrKevinWeiss <[email protected]>

aabadie · 2023-05-24T09:33:42Z

bors cancel

bors · 2023-05-24T09:33:44Z

Canceled.

aabadie · 2023-05-24T09:45:30Z

bors merge

18005: pkg/semtech-loramac: model in Kconfig r=aabadie a=aabadie 19576: boards: add stm32l496g-disco support r=aabadie a=gschorcht ### Contribution description The PR adds the board definition for the STM2L496G-DISO board. It is the same board that is also shipped with the P-L496G-CELL02 LTE pack for which we already have the board definition `p-l496g-cell02`. However, `stm32l496g-disco` provides a complete configuration of the board and supports the following features in addition to `p-l496g-cell02`: ``` > FEATURES_PROVIDED += periph_adc > FEATURES_PROVIDED += periph_dac > FEATURES_PROVIDED += periph_dma > FEATURES_PROVIDED += periph_pwm > FEATURES_PROVIDED += periph_uart_hw_fc > FEATURES_PROVIDED += arduino ``` In the long term, `p-l496g-cell02` is to be based on the new full `stm32l496g-disco` board definition. The CPT and the LCD display are not yet supported since they are connected to/controlled by the MFX (a STM32L152-based sub-system) and the FMC peripheral. ### Testing procedure All basic tests should work with the new board definition. The following tests were executed and did succeed: - [x] `tests/periph/adc` - [x] `tests/periph/dac` - [x] `tests/periph/i2c` for `I2C_DEV(0)`, `I2C_DEV(1)` is not exposed and not tested - [x] `tests/periph/pwm` - [x] `tests/periph/spi` for `SPI_DEV(0)`, `SPI_DEV(1) connection not soldered and not tested - [x] `tests/periph/timer` for `TIMER_DEV(0)` and `TIMER_DEV(1)` - [x] `tests/periph/uart` for `UART_DEV(0)`, `UART_DEV(1)` and `UART_DEV(2)` - [x] `tests/usbus_cdc_ecm` together with `stdio_cdc_acm` ### Issues/PRs references ~Depends on PR #19571~ ~Depends on PR #19572~ ~Depends on PR #19573~ 19650: drivers/nrf24l01p: model in kconfig r=aabadie a=aabadie 19660: cpu/rpx0xx: Fix kconfig model r=aabadie a=MrKevinWeiss ### Contribution description Broken master due to incorrect model of the periph_pio in kconfig. ### Testing procedure Green murdock (now that the board is added to the list) ### Issues/PRs references Look at the master CI... Co-authored-by: Alexandre Abadie <[email protected]> Co-authored-by: Gunar Schorcht <[email protected]> Co-authored-by: MrKevinWeiss <[email protected]>

bors · 2023-05-24T10:00:16Z

Build failed (retrying...):

Murdock

aabadie · 2023-05-24T11:50:33Z

bors cancel

bors · 2023-05-24T11:50:35Z

Canceled.

aabadie · 2023-05-24T11:50:50Z

bors merge

aabadie · 2023-05-24T12:21:54Z

bors cancel

Some concerns about ztimer are arising in private.

bors · 2023-05-24T12:21:56Z

Canceled.

MrKevinWeiss · 2023-05-24T14:15:12Z

Maybe I will try different PR that will only deal with the problems with ztimer, it is out of scope of this PR and should probably be blocklisted or wait until the ztimer pr is done.

aabadie · 2024-09-27T13:04:47Z

This PR is no longer needed

aabadie requested review from MichelRottleuthner, cgundogan, fjmolinas, jia200x, leandrolanzieri and smlng as code owners April 25, 2022 16:35

github-actions bot added Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: LoRa Area: LoRa radio support Area: pkg Area: External package ports Area: sys Area: System Area: tests Area: tests and testing framework labels Apr 25, 2022

aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 25, 2022

leandrolanzieri reviewed Apr 26, 2022

View reviewed changes

aabadie commented Apr 26, 2022

View reviewed changes

drivers/sx127x/Kconfig Outdated Show resolved Hide resolved

aabadie force-pushed the pr/pkg/semtech-loramac-kconfig branch 2 times, most recently from 3ca8520 to c60bebb Compare April 26, 2022 09:37

MrKevinWeiss requested changes Apr 26, 2022

View reviewed changes

github-actions bot added the Area: examples Area: Example Applications label Apr 26, 2022

aabadie mentioned this pull request Apr 27, 2022

pkg/nanors: model in kconfig #18024

Merged

aabadie force-pushed the pr/pkg/semtech-loramac-kconfig branch from 2e2281c to 2004b3e Compare April 27, 2022 09:53

github-actions bot removed the Area: drivers Area: Device drivers label Apr 27, 2022

MrKevinWeiss reviewed Apr 28, 2022

View reviewed changes

aabadie force-pushed the pr/pkg/semtech-loramac-kconfig branch from 763f175 to f5c3448 Compare May 5, 2022 19:40

aabadie requested a review from kaspar030 as a code owner May 6, 2022 08:48

github-actions bot added the Area: CI Area: Continuous Integration of RIOT components label May 6, 2022

aabadie 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 May 6, 2022

aabadie force-pushed the pr/pkg/semtech-loramac-kconfig branch from f3f7720 to 0ec135b Compare May 22, 2023 08:41

tests/sys/fido2_ctap: force periph_timer backend for ztimer

63a6e6a

MrKevinWeiss approved these changes May 24, 2023

View reviewed changes

bors bot added a commit that referenced this pull request May 24, 2023

Merge #18005

b141d3b

18005: pkg/semtech-loramac: model in Kconfig r=MrKevinWeiss a=aabadie Co-authored-by: Alexandre Abadie <[email protected]>

aabadie mentioned this pull request May 24, 2023

boards: add stm32l496g-disco support #19576

Merged

8 tasks

MrKevinWeiss mentioned this pull request May 30, 2023

sys/ztimer: Simplify ztimer backend selection #19684

Open

aabadie closed this Sep 27, 2024

aabadie deleted the pr/pkg/semtech-loramac-kconfig branch September 27, 2024 13:04

	select MODULE_PERIPH_RTC if HAS_PERIPH_RTC
	select MODULE_PERIPH_RTC if HAS_PERIPH_RTC && !HAVE_SHARED_PERIPH_RTT_PERIPH_RTC

Conversation

aabadie commented Apr 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

leandrolanzieri Apr 26, 2022

Choose a reason for hiding this comment

Uh oh!

aabadie Apr 26, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MrKevinWeiss left a comment

Choose a reason for hiding this comment

Uh oh!

aabadie commented Apr 26, 2022

Uh oh!

aabadie commented Apr 27, 2022

Uh oh!

MrKevinWeiss left a comment

Choose a reason for hiding this comment

Uh oh!

MrKevinWeiss Apr 28, 2022

Choose a reason for hiding this comment

Uh oh!

aabadie commented May 22, 2023

Uh oh!

MrKevinWeiss commented May 22, 2023

Uh oh!

MrKevinWeiss commented May 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MrKevinWeiss commented May 22, 2023

Uh oh!

aabadie commented May 23, 2023

Uh oh!

MrKevinWeiss left a comment

Choose a reason for hiding this comment

Uh oh!

MrKevinWeiss commented May 24, 2023

Uh oh!

aabadie commented May 24, 2023

Uh oh!

bors bot commented May 24, 2023

Uh oh!

aabadie commented May 24, 2023

Uh oh!

aabadie commented May 24, 2023

Uh oh!

bors bot commented May 24, 2023

Uh oh!

aabadie commented May 24, 2023

Uh oh!

bors bot commented May 24, 2023

Uh oh!

aabadie commented May 24, 2023

Uh oh!

bors bot commented May 24, 2023

Uh oh!

aabadie commented May 24, 2023

Uh oh!

aabadie commented May 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors bot commented May 24, 2023

Uh oh!

MrKevinWeiss commented May 24, 2023

Uh oh!

aabadie commented Sep 27, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aabadie commented Apr 25, 2022 •

edited

Loading

MrKevinWeiss commented May 22, 2023 •

edited

Loading

aabadie commented May 24, 2023 •

edited

Loading