Skip to content

modem: modules: remove EXPERIMENTAL #93435

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

modem: modules: remove EXPERIMENTAL #93435

wants to merge 2 commits into from
+24 −2

Conversation

bjarki-andreasen
Copy link
Contributor

Remove the EXPERIMENTAL label from the MODEM_MODULES. These modules are fully tested and widely used, and have been present for 2 years.

@zephyrbot zephyrbot added area: Modem size: XS A PR changing only a single line of code labels Jul 21, 2025
@zephyrbot zephyrbot requested a review from tomi-font July 21, 2025 11:27
ubieda
ubieda reviewed Jul 21, 2025
View reviewed changes
Copy link
Member

@ubieda ubieda left a comment

Choose a reason for hiding this comment

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

Should we add a version to the APIs under include/zephyr/modem?

@bjarki-andreasen
Copy link
Contributor Author

Should we add a version to the APIs under include/zephyr/modem?

We can, but I don't think we track the APIs of subsystems :)

ubieda
ubieda previously approved these changes Jul 21, 2025
View reviewed changes
@kartben
Copy link
Contributor

kartben commented Jul 21, 2025 •
edited
Loading

Should we add a version to the APIs under include/zephyr/modem?

We can, but I don't think we track the APIs of subsystems :)

Mmmh wait, what? :)

So yes, please :)

@bjarki-andreasen
Copy link
Contributor Author

Should we add a version to the APIs under include/zephyr/modem?

We can, but I don't think we track the APIs of subsystems :)

Mmmh wait, what? :)

So yes, please :)

Its been a long time since I added that :D I just looked at headers in tree to find many many which don't mention a version :)

@kartben
Copy link
Contributor

kartben commented Jul 21, 2025

Its been a long time since I added that :D I just looked at headers in tree to find many many which don't mention a version :)

:)
Yeah I think it would make sense for all "sub"-headers to also have an explicit version set btw

@bjarki-andreasen bjarki-andreasen force-pushed the modem-remove-experimental branch from 562c353 to 32522f2 Compare July 21, 2025 12:26
fabiobaltieri
fabiobaltieri previously approved these changes Jul 21, 2025
View reviewed changes
tomi-font
tomi-font reviewed Jul 21, 2025
View reviewed changes
Copy link
Contributor

@tomi-font tomi-font left a comment

Choose a reason for hiding this comment

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

4.3, not 4.2?

@kartben
Copy link
Contributor

kartben commented Jul 21, 2025 •
edited
Loading

4.3, not 4.2?

Neither -- the @since should represent when the API was introduced so shouldn't be updated

nordicjm
nordicjm previously approved these changes Jul 21, 2025
View reviewed changes
rerickson1
rerickson1 previously approved these changes Jul 21, 2025
View reviewed changes
@bjarki-andreasen
modem: modules: remove EXPERIMENTAL
155a407 
Remove the EXPERIMENTAL label from the MODEM_MODULES and update
API version to 1.0.0. These modules are fully tested and widely
used, and have been present for 2 years.

Signed-off-by: Bjarki Arge Andreasen <[email protected]>
@bjarki-andreasen bjarki-andreasen force-pushed the modem-remove-experimental branch from d153ed9 to efb5471 Compare July 21, 2025 14:32
@bjarki-andreasen
Copy link
Contributor Author

Updated @since to reflect when APIs where introduced instead of when the version was updated

kartben
kartben previously approved these changes Jul 21, 2025
View reviewed changes
tomi-font
tomi-font reviewed Jul 21, 2025
View reviewed changes
include/zephyr/modem/ubx.h Outdated
@@ -23,6 +23,8 @@ extern "C" {
/**
* @brief Modem Ubx
* @defgroup modem_ubx Modem Ubx
* @since 4.2
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem to be from 4.2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

corrected to v3.6

@bjarki-andreasen
modem: add versions to all headers
bc14490 
Versions for specific modem modules don't include versions in their
headers, and the chat module did not even define a group.

This commits updates most modem modules to include versions in
their headers. Specifically, the following modules have their
versions set to 1.0.0, as these should no longer be marked
experimental:

- modem_chat
- modem_cmux
- modem_pipe
- modem_pipelink
- modem_ppp

The modem_ubx module was just refactored quite a bit so should be
kept experimental for a while. Thus its version is set to 0.1.0.

Signed-off-by: Bjarki Arge Andreasen <[email protected]>
@bjarki-andreasen bjarki-andreasen force-pushed the modem-remove-experimental branch from efb5471 to bc14490 Compare July 23, 2025 10:41
@bjarki-andreasen bjarki-andreasen removed the size: XS A PR changing only a single line of code label Jul 23, 2025
tomi-font
tomi-font reviewed Jul 23, 2025
View reviewed changes
include/zephyr/modem/ubx.h
@@ -23,6 +23,8 @@ extern "C" {
/**
* @brief Modem Ubx
* @defgroup modem_ubx Modem Ubx
* @since 3.6
* @version 0.1.0
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 is not 1.0 then MODEM_UBX should select EXPERIMENTAL?

tomi-font
tomi-font reviewed Jul 23, 2025
View reviewed changes
include/zephyr/modem/ubx.h
@@ -23,6 +23,8 @@ extern "C" {
/**
* @brief Modem Ubx
* @defgroup modem_ubx Modem Ubx
* @since 3.6
Copy link
Contributor

@tomi-font tomi-font Jul 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

BTW looking at the git log this should be 3.7 (it's the first release with this)

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomi-font tomi-font tomi-font left review comments

@kartben kartben kartben left review comments

@fabiobaltieri fabiobaltieri fabiobaltieri left review comments

@rerickson1 rerickson1 rerickson1 left review comments

@nordicjm nordicjm nordicjm left review comments

@ubieda ubieda ubieda left review comments

@carlescufi carlescufi Awaiting requested review from carlescufi

@gmarull gmarull Awaiting requested review from gmarull

@nashif nashif Awaiting requested review from nashif

At least 2 approving reviews are required to merge this pull request.

Assignees

@bjarki-andreasen bjarki-andreasen

Labels
area: Documentation Infrastructure area: Modem
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@bjarki-andreasen @kartben @fabiobaltieri @rerickson1 @nordicjm @ubieda @tomi-font @zephyrbot