Skip to content
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

drivers: modem: add Telit ME910G1 #63711

Merged
merged 4 commits into from
Nov 2, 2023
Merged

drivers: modem: add Telit ME910G1 #63711

merged 4 commits into from
Nov 2, 2023

Conversation

jeffwelder-ellenbytech
Copy link
Contributor

Added Telit ME910G1 dts and modem chat scripts.

Refactor device macro to be custom for each modem to allow custom configurations on each modem (assisted by @bjarki-trackunit).
(This was needed by Telit ME910G1 to allow for ON_OFF to be toggled for minimum of 5 seconds as per Telit)

@github-actions
Copy link

github-actions bot commented Oct 9, 2023

Hello @jeffwelder-ellenbytech, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

@jeffwelder-ellenbytech jeffwelder-ellenbytech marked this pull request as ready for review October 9, 2023 18:41
@zephyrbot zephyrbot added area: Modem area: Devicetree Binding PR modifies or adds a Device Tree binding labels Oct 9, 2023
@bjarki-andreasen
Copy link
Collaborator

Looks good :) FYI it will take a while to get merged because of the feature freeze :)

@jeffwelder-ellenbytech
drivers: modem: Individualize Modem Device Macros
fd7b75c 
Refactor device macro to be custom for each modem
to allow custom configurations.

Signed-off-by: Jeff Welder <[email protected]>
@jeffwelder-ellenbytech jeffwelder-ellenbytech force-pushed the feature_add_modem_telit_me910g1 branch 2 times, most recently from 590d03b to b60d0bf Compare October 10, 2023 14:41
bjarki-andreasen
bjarki-andreasen previously approved these changes Oct 10, 2023
View reviewed changes
Copy link
Collaborator

@bjarki-andreasen bjarki-andreasen left a comment

Choose a reason for hiding this comment

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

Nice work :)

@bjarki-andreasen
Copy link
Collaborator

@rerickson1 Could you review this :)

ycsin
ycsin reviewed Oct 25, 2023
View reviewed changes
Copy link
Member

@ycsin ycsin left a comment

Choose a reason for hiding this comment

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

From the looks of it, it seems that modem_cellular.c can grow indefinitely, as the number of supported modem grows?

fabiobaltieri
fabiobaltieri reviewed Oct 25, 2023
View reviewed changes
dts/bindings/modem/telit,me910g1.yaml Outdated
@@ -0,0 +1,23 @@
# Copyright(c) 2023 Jeff Welder(Ellenby Technologies, Inc.)
# SPDX - License - Identifier : Apache - 2.0
Copy link
Member

Choose a reason for hiding this comment

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

This probably wants to be SPDX-License-Identifier: Apache-2.0

Copy link
Member

Choose a reason for hiding this comment

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

@jeffwelder-ellenbytech please fix this.

@bjarki-andreasen
Copy link
Collaborator

bjarki-andreasen commented Oct 25, 2023 •
edited
Loading

@ycsin The driver is capable of supporing an "infinite" amount of modems, as long as said modems implement the standard AT commands, CMUX and PPP. However, the driver is an MVP, so it will not get any fancy non generic features, like setting radio access tech, sending sms, internal gnss, etc. so it should not grow in complexity :)

@jeffwelder-ellenbytech
dts: bindings: Add Telit ME910G1 Modem
cabe9c2 
Add Telit to vendor-prefixes, and add ME910G1 bindings

Signed-off-by: Jeff Welder <[email protected]>
@jeffwelder-ellenbytech
Copy link
Contributor Author

Fixed the licensing.
Working on an improved chat script that works with various eSIMs before merge, will commit again once that is complete.

ycsin
ycsin previously approved these changes Oct 27, 2023
View reviewed changes
@bjarki-andreasen
Copy link
Collaborator

Fixed the licensing. Working on an improved chat script that works with various eSIMs before merge, will commit again once that is complete.

Love it! I have added the DNM (Do Not Merge) label until you merge the fixes, you can remove it and re-request reviews when ready :)

@bjarki-andreasen bjarki-andreasen added the DNM This PR should not be merged (Do Not Merge) label Oct 27, 2023
@jeffwelder-ellenbytech
drivers: modem: modem_cellular: add Telit ME910G1
8fb1d8c 
Added chat script and initial config to support Telit ME910G1.
Needed to edit power_pulse pin timing in accordance to datasheet.

Signed-off-by: Jeff Welder <[email protected]>
@jeffwelder-ellenbytech
tests: build_all: modem: Add Telit ME910G1 Test Coverage
e4477ae 
Added bindings to build_all test for new Telit ME910G1 Modem.

Signed-off-by: Jeff Welder <[email protected]>
@jeffwelder-ellenbytech
Copy link
Contributor Author

jeffwelder-ellenbytech commented Nov 1, 2023 •
edited
Loading

I updated the chat scripts to have a more stable experience with the modem. Specifically I was having trouble with Verizon/AT&T eSIMs either giving an error or taking unusually long for setting the APN.

Init Script:

  • Added PDP Context Setting (Allows retry on error by restarting init script)

Dial Script:

  • Remove PDP Context Activation (Note: second parameter not supported by ME910G1 by current firmware)
  • Remove PDP Context Setting (Done in init script to allow error retries)
  • Added Dummy AT command (Dial script doesn't work if it is only 1 command long)

Also changed power on/off pulse in accordance to Telit's guidance.

If the network is too fast to register, the state machine in the dial script will miss the unsolicited registration status, and hang forever. This is fixed in @bjarki-trackunit 's #64659 by polling the registration state which I tested this morning as working. 😄

Also think I need someone to remove DNM label.

@ycsin ycsin removed the DNM This PR should not be merged (Do Not Merge) label Nov 1, 2023
@fabiobaltieri fabiobaltieri merged commit 9c51a26 into zephyrproject-rtos:main Nov 2, 2023
22 of 23 checks passed
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 2, 2023

Hi @jeffwelder-ellenbytech!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

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

@fabiobaltieri fabiobaltieri fabiobaltieri approved these changes

@rerickson1 rerickson1 rerickson1 approved these changes

@bjarki-andreasen bjarki-andreasen bjarki-andreasen approved these changes

@galak galak Awaiting requested review from galak

@nashif nashif Awaiting requested review from nashif

@ycsin ycsin Awaiting requested review from ycsin

Assignees

@rerickson1 rerickson1

Labels
area: Devicetree Binding PR modifies or adds a Device Tree binding area: Modem
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jeffwelder-ellenbytech @bjarki-andreasen @fabiobaltieri @ycsin @rerickson1 @zephyrbot