Skip to content

ports/esp32: Updated network_ppp.c to match current documentation #17008

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.

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thokis
Copy link

@thokis thokis commented Mar 25, 2025

Summary

This MR updates the network.PPP module of the ESP32 port to match the current documentation. The status() method should behave like the common LwIP implementation and the poll() method will raise an RuntimeError indicating that this is not really supported on this port. Also did some minor resorting to match the common LwIP implementation.

This would solve #16998

Testing

Tested on ESP32_GENERIC and ESP32_GENERIC_S3 boards with SIMCOM800L and SIMCOM7080G modules. At least with ESP32_GENERIC the PPP connection is connected at least 18 hours.
Screenshot from 2025-03-25 20-35-16-obfuscated

@thokis thokis force-pushed the feature/esp32_ppp_updated branch from f37067e to e6843e7 Compare March 25, 2025 19:41
@thokis thokis changed the title ports: esp32: Updated network_ppp.c to match current documentation ports/esp32: Updated network_ppp.c to match current documentation Mar 25, 2025
@thokis thokis force-pushed the feature/esp32_ppp_updated branch from e6843e7 to c25e606 Compare March 25, 2025 19:51
This commit updates the network.PPP module of the ESP32 port to match
the current documentation. The the status() method should behave like
the "common" LwIP implementation and the poll() method will raise an
RuntimeError indicating that this is not really supported on this port.

Signed-off-by: Thomas Kiss <thomas.kiss@lemonbeat.com>
@DvdGiessen
Copy link
Contributor

I worked on similar changes previous year and finally submitted them as a PR last week, see #17138. That fixes a few crashes I was having, and as a side-effect also does address the your linked issue about the documentation not matching. Linking it here since you might find it interesting.

Note these two PR's conflict. In the other PR I replaced parts of the ESP32 version with the extmod version. So for example it uses IRQ/poll() system from the extmod version for reading data from the UART and completely removes the task previously used by the ESP32 version. Whereas this PR one adds additional enhancements to the task management logic, and adds explicit warnings about the poll() method being unsupported.

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.

3 participants