Skip to content

Conversation

lgehreke
Copy link
Contributor

The sim 7080 driver has a lot functionality crammed into one source file. This makes it hard to maintain. Furthermore some bugs were fixed during the refactor.

Changes:

  • Split modem driver into multiple files
  • One file per modem functionality
  • Verifying xtra file validity on aided gps
  • Modem driver uses fixed baudrate to evade autobaud sequence

Additions:

  • Added multiple boot modes
  • Added force reset function
  • Added function to query time
  • Added battery measurement function
  • Added function to query iccid
  • Added gpio set function

Bugfixes:

  • Corrected socket connect behavior
  • Fixed wrong pdp context on offload_connect
  • Send sequence queries the available tx space before sending

rerickson1
rerickson1 previously approved these changes Aug 26, 2025
Copy link
Member

@rerickson1 rerickson1 left a comment

Choose a reason for hiding this comment

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

Do a rebase and see if CI is fixed.

@lgehreke lgehreke force-pushed the sim7080-refactor branch 4 times, most recently from 3981738 to f7cc080 Compare August 26, 2025 11:38
@lgehreke lgehreke requested a review from rerickson1 August 26, 2025 12:37
@tomi-font tomi-font removed their request for review August 27, 2025 12:06
@rerickson1
Copy link
Member

Please fix compliance issues.

@lgehreke
Copy link
Contributor Author

lgehreke commented Sep 3, 2025

Please fix compliance issues.

Is this a bug in the compliance checks? Why should there be spaces around the ':' of a goto label? This is used in multiple occasions but the pipeline fails only at this line.

@bjarki-andreasen
Copy link
Contributor

bjarki-andreasen commented Sep 3, 2025

Please fix compliance issues.

Is this a bug in the compliance checks? Why should there be spaces around the ':' of a goto label? This is used in multiple occasions but the pipeline fails only at this line.

it seems to be a bug in the CI check since the label "error" is used, just use another label like out or something

@rerickson1
Copy link
Member

Please fix compliance issues.

Is this a bug in the compliance checks? Why should there be spaces around the ':' of a goto label? This is used in multiple occasions but the pipeline fails only at this line.

Also, fix trailing spaces.

@lgehreke
Copy link
Contributor Author

Please fix compliance issues.

Is this a bug in the compliance checks? Why should there be spaces around the ':' of a goto label? This is used in multiple occasions but the pipeline fails only at this line.

it seems to be a bug in the CI check since the label "error" is used, just use another label like out or something

Error is used in the same file as goto label in other locations. Maybe its the preceeding ifdef?

@lgehreke
Copy link
Contributor Author

Please fix compliance issues.

Is this a bug in the compliance checks? Why should there be spaces around the ':' of a goto label? This is used in multiple occasions but the pipeline fails only at this line.

it seems to be a bug in the CI check since the label "error" is used, just use another label like out or something

Compliance checks are still failing with new label name. Any ideas on how to fix this?

@lgehreke lgehreke force-pushed the sim7080-refactor branch 2 times, most recently from 08fa1da to 378e5f9 Compare September 15, 2025 07:39
@lgehreke
Copy link
Contributor Author

Please fix compliance issues.

Is this a bug in the compliance checks? Why should there be spaces around the ':' of a goto label? This is used in multiple occasions but the pipeline fails only at this line.

Also, fix trailing spaces.

Fixed in this commit f7cc080 but pipeline still complains. Any thoughts on that?

@bjarki-andreasen
Copy link
Contributor

Please fix compliance issues.

Is this a bug in the compliance checks? Why should there be spaces around the ':' of a goto label? This is used in multiple occasions but the pipeline fails only at this line.

Also, fix trailing spaces.

Fixed in this commit f7cc080 but pipeline still complains. Any thoughts on that?

trailing whitespace are tabs and spaces following trailing lines added, you should be able to enable whitespace rendering in your IDE so you can see them, I added a traling space below to show what its complaining about.

int my_line = 0 

IDEs like vscode have a "trim trailing whitespace" option which will do this for you when you save the file

@lgehreke
Copy link
Contributor Author

Please fix compliance issues.

Is this a bug in the compliance checks? Why should there be spaces around the ':' of a goto label? This is used in multiple occasions but the pipeline fails only at this line.

Also, fix trailing spaces.

Fixed in this commit f7cc080 but pipeline still complains. Any thoughts on that?

trailing whitespace are tabs and spaces following trailing lines added, you should be able to enable whitespace rendering in your IDE so you can see them, I added a traling space below to show what its complaining about.

int my_line = 0 

IDEs like vscode have a "trim trailing whitespace" option which will do this for you when you save the file

I already used vscodes trailing whitespace feature on the files mentioned by the pipeline. There are no changes. Is the number before the error a commit hash? If so they dont exist anymore

@lgehreke
Copy link
Contributor Author

Please fix compliance issues.

Is this a bug in the compliance checks? Why should there be spaces around the ':' of a goto label? This is used in multiple occasions but the pipeline fails only at this line.

Also, fix trailing spaces.

Fixed in this commit f7cc080 but pipeline still complains. Any thoughts on that?

trailing whitespace are tabs and spaces following trailing lines added, you should be able to enable whitespace rendering in your IDE so you can see them, I added a traling space below to show what its complaining about.

int my_line = 0 

IDEs like vscode have a "trim trailing whitespace" option which will do this for you when you save the file

Running git rebase main --whitespace=fix resolved the issue

Copy link
Contributor

Choose a reason for hiding this comment

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

please use doxygen comments (/** ... */) for everything that's public API. Please also add comments where missing. See https://builds.zephyrproject.io/zephyr/pr/94929/api-coverage/zephyr/drivers/modem/simcom-sim7080.h.gcov.html

Placed sim7080 driver in separate directory and split it in multiple
source files for better readability.

Signed-off-by: Lukas Gehreke <[email protected]>
Network and gnss can be disabled with stop functions instead of power
cycling the modem. The start functions will also not power cycle the
modem. In order to call the start functions the modem needs to be
booted.

Signed-off-by: Lukas Gehreke <[email protected]>
Moved socket related urc handling to dedicated file.

Signed-off-by: Lukas Gehreke <[email protected]>
Socket functions return EINVAL instead of EAGAIN when modem is
not in networking mode. Using EAGAIN could lead to a infinite sleep
loop in offload_sendmsg().

Signed-off-by: Lukas Gehreke <[email protected]>
Removed sleep to prevent infinite loop in sendmsg.

Signed-off-by: Lukas Gehreke <[email protected]>
Connection id and pdp index were swapped. Corrected this and
hardcoded pdp index since 0 is always used as index.

Signed-off-by: Lukas Gehreke <[email protected]>
Query available tx size before sending data instead of using
hardcoded number. Removed unnecessary strg+z send after data.

Signed-off-by: Lukas Gehreke <[email protected]>
The modem handler error can not be used to transport the result
of CAOPEN since it is overwritten by the trailing OK.
Socket does not get closed forcefully when connecting failed.

Signed-off-by: Lukas Gehreke <[email protected]>
Removed connected check from offload_close in order to be able
to close non-connected sockets.

Signed-off-by: Lukas Gehreke <[email protected]>
If a socket gets closed by the network while waiting on data
in offload_recvfrom this would hangup forever since modem_socket_wait_data
does not support timeouts. When the socket gets closed this wait is
unblocked.

Signed-off-by: Lukas Gehreke <[email protected]>
Added functionality to set modem gpio pins.

Signed-off-by: Lukas Gehreke <[email protected]>
Added function to get the iccid number.

Signed-off-by: Lukas Gehreke <[email protected]>
Added function to measure battery voltage, battery charge status
and battery connection level.

Signed-off-by: Lukas Gehreke <[email protected]>
Added function to query the modem state from application side.

Signed-off-by: Lukas Gehreke <[email protected]>
Added command to query ue system information from the modem.

Signed-off-by: Lukas Gehreke <[email protected]>
Added funtions to download a gnss xtra file, query its validity
and use it in gps.

Signed-off-by: Lukas Gehreke <[email protected]>
Added funtion to query local time and added injection time to
gnss xtra validity query function.

Signed-off-by: Lukas Gehreke <[email protected]>
Starting gnss with xtra functionality is only possible if the
validity of the xtra file was queried. Enabling xtra will be skipped
if the file is not valid

Signed-off-by: Lukas Gehreke <[email protected]>
Added function to forcefully reset the modem by holding the pwrkey
for 15 seconds.

Signed-off-by: Lukas Gehreke <[email protected]>
Added check that makes sure ftp works when the network context is
already active.

Signed-off-by: Lukas Gehreke <[email protected]>
Fixed zephyr style violations.

Signed-off-by: Lukas Gehreke <[email protected]>
Removed label because the compliance checks fail.

Signed-off-by: Lukas Gehreke <[email protected]>
@lgehreke lgehreke force-pushed the sim7080-refactor branch 2 times, most recently from a299d52 to d2d9ead Compare October 13, 2025 11:09
Added missing doxygen comments to public header.

Signed-off-by: Lukas Gehreke <[email protected]>
Timeout and retries for DNS lookups were hardcoded. This commit
introduces kconfig settings for the default values and functions
for runtime configuration.

Signed-off-by: Lukas Gehreke <[email protected]>
Copy link
Contributor

@kartben kartben left a comment

Choose a reason for hiding this comment

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

+1 for docs, thanks for your efforts on this, much appreciated!

Copy link

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants