PPP: add "battery status" read stub functions #10043

WebDust21 · 2024-07-17T15:08:23Z

Description of Change

Add Arduino stub functions to access the functionality provided by the underlying ESP-MODEM "esp_modem_get_battery_status" call
Replaced verbose checks on each Arduino stub function call with a single succint #define for ease of code maintainability.

Tests scenarios

Tested on an ESP32-S2 with a SIM7080G modem. Love the PPP functionality, it just works! Just providing some functional extensibility to functionality that is already present.

    testClient("google.com", 80);
    Serial.print("RSSI: "); Serial.println(PPP.RSSI());
    Serial.print("BtyVolt: "); Serial.println(PPP.getBatteryVoltage());

output result

[...]
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
Connection Success
RSSI: 18
BtyVolt: 3846

showing that the modem VCC power supply is 3.846v.

Related links

ESP-MODEM "esp_modem_get_battery_status": https://github.com/espressif/esp-protocols/blob/906e447193710c1772305a218a9d0b315a549de4/components/esp_modem/src/esp_modem_c_api.cpp#L280-L293

Initially I thought that "esp_modem_get_battery_status" was leveraging AT+CADC, but it's not: it's using AT+CBC: https://github.com/espressif/esp-protocols/blob/906e447193710c1772305a218a9d0b315a549de4/components/esp_modem/src/esp_modem_command_library.cpp#L207-L234

replaced all function protections with a #define to simplify code functionality. Also added "getBattery" functions to leverage "esp_modem_get_battery_status" call in ESP-MODEM.

CLAassistant · 2024-07-17T15:08:29Z

All committers have signed the CLA.

github-actions · 2024-07-17T15:09:01Z

	Warnings
⚠️	Some issues found for the commit messages in this PR: the commit message `"add "getBattery" function defs to PPP.h"`: summary looks empty type/action looks empty the commit message `"add "getBattery" functions + #define simplicity"`: body's lines must not be longer than 100 characters summary looks empty type/action looks empty Please fix these commit messages - here are some basic tips: follow Conventional Commits style correct format of commit message should be: `<type/action>(<scope/component>): <summary>`, for example `fix(esp32): Fixed startup timeout issue` allowed types are: `change,ci,docs,feat,fix,refactor,remove,revert,test` sufficiently descriptive message summary should be between 20 to 72 characters and start with upper case letter avoid Jira references in commit messages (unavailable/irrelevant for our customers) `TIP:` Install pre-commit hooks and run this check when committing (uses the Conventional Precommit Linter).

	Messages
📖	You might consider squashing your 5 commits (simplifying branch history).

👋 Hello WebDust21, we appreciate your contribution to this project!

Click to see more instructions ...

This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
- Addressing info messages (📖) is strongly recommended; they're less critical but valuable.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...

We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against 81f9e54

github-actions · 2024-07-17T15:12:52Z

Memory usage test (comparing PR against master branch)

The table below shows the summary of memory usage change (decrease - increase) in bytes and percentage for each target.

Memory	FLASH [bytes]		FLASH [%]		RAM [bytes]		RAM [%]
Target	DEC	INC	DEC	INC	DEC	INC	DEC	INC
ESP32S3	0	⚠️ +84	0.00	⚠️ +0.02	0	0	0.00	0.00
ESP32S2	0	⚠️ +84	0.00	⚠️ +0.02	0	0	0.00	0.00
ESP32C3	0	0	0.00	0.00	0	0	0.00	0.00
ESP32C6	0	0	0.00	0.00	0	0	0.00	0.00
ESP32H2	0	0	0.00	0.00	0	0	0.00	0.00
ESP32	0	⚠️ +84	0.00	⚠️ +0.02	0	0	0.00	0.00

Click to expand the detailed deltas report [usage change in BYTES]

Target	ESP32S3		ESP32S2		ESP32C3		ESP32C6		ESP32H2		ESP32
Example	FLASH	RAM	FLASH	RAM	FLASH	RAM	FLASH	RAM	FLASH	RAM	FLASH	RAM
PPP/examples/PPP_Basic	⚠️ +84	0	⚠️ +84	0	0	0	0	0	0	0	⚠️ +84	0

github-actions · 2024-07-17T15:15:43Z

Test Results

56 files ±0 56 suites ±0 4m 50s ⏱️ -7s
21 tests ±0 21 ✅ ±0 0 💤 ±0 0 ❌ ±0
135 runs ±0 135 ✅ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 81f9e54. ± Comparison against base commit 690bdb5.

♻️ This comment has been updated with latest results.

libraries/PPP/src/PPP.h

me-no-dev · 2024-07-17T15:31:08Z

libraries/PPP/src/PPP.cpp

+  int volt, bcs, bcl;
+  esp_err_t err = esp_modem_get_battery_status(_dce, volt, bcs, bcl);
+  if (err != ESP_OK) {
+    //log_e("esp_modem_get_battery_status failed with %d %s", err, esp_err_to_name(err));


why are the log lines commented out?

Well, I copied the functionality from other functions in PPP.cpp (RSSI + BER), which had them commented out:

arduino-esp32/libraries/PPP/src/PPP.cpp

Line 515 in 690bdb5

// log_e("esp_modem_get_signal_quality failed with %d %s", err, esp_err_to_name(err));

arduino-esp32/libraries/PPP/src/PPP.cpp

Line 534 in 690bdb5

// log_e("esp_modem_get_signal_quality failed with %d %s", err, esp_err_to_name(err));

I guess I'm not sure why those two are commented out...when none of the others in the entire file are.

That's literally the only reason I commented them out...

the were commented out because they were causing floods during normal operation. If you expect the battery functions to often fail, you can leave them commented

LOL, OK...well...I uncommented those two lines referenced above in the latest commit...oops?

I would not expect those commands to be "frequent failures", but maybe they are problematic. No issues on my end though.

WebDust21 added 2 commits July 17, 2024 10:54

add "getBattery" functions + #define simplicity

3afa863

replaced all function protections with a #define to simplify code functionality. Also added "getBattery" functions to leverage "esp_modem_get_battery_status" call in ESP-MODEM.

add "getBattery" function defs to PPP.h

81f98e5

me-no-dev reviewed Jul 17, 2024

View reviewed changes

libraries/PPP/src/PPP.h Outdated Show resolved Hide resolved

me-no-dev reviewed Jul 17, 2024

View reviewed changes

WebDust21 added 2 commits July 17, 2024 12:19

fix(ppp): uncomment error logging, change new function naming

055be33

fix(ppp): rename new functions for naming conformity

c41b4d8

me-no-dev approved these changes Jul 17, 2024

View reviewed changes

VojtechBartoska added the Area: Libraries Issue is related to Library support. label Jul 18, 2024

me-no-dev added the Status: Pending Merge Pull Request is ready to be merged label Jul 19, 2024

ci(pre-commit): Apply automatic fixes

81f9e54

me-no-dev merged commit 0fa4aa6 into espressif:master Jul 22, 2024
40 checks passed

WebDust21 deleted the ppp-add-battery-status branch July 23, 2024 16:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PPP: add "battery status" read stub functions #10043

PPP: add "battery status" read stub functions #10043

WebDust21 commented Jul 17, 2024

CLAassistant commented Jul 17, 2024 •

edited

Loading

github-actions bot commented Jul 17, 2024 •

edited

Loading

github-actions bot commented Jul 17, 2024

github-actions bot commented Jul 17, 2024 •

edited

Loading

me-no-dev Jul 17, 2024

WebDust21 Jul 17, 2024

me-no-dev Jul 17, 2024

WebDust21 Jul 17, 2024

PPP: add "battery status" read stub functions #10043

PPP: add "battery status" read stub functions #10043

Conversation

WebDust21 commented Jul 17, 2024

Description of Change

Tests scenarios

Related links

CLAassistant commented Jul 17, 2024 • edited Loading

github-actions bot commented Jul 17, 2024 • edited Loading

github-actions bot commented Jul 17, 2024

Memory usage test (comparing PR against master branch)

github-actions bot commented Jul 17, 2024 • edited Loading

Test Results

me-no-dev Jul 17, 2024

Choose a reason for hiding this comment

WebDust21 Jul 17, 2024

Choose a reason for hiding this comment

me-no-dev Jul 17, 2024

Choose a reason for hiding this comment

WebDust21 Jul 17, 2024

Choose a reason for hiding this comment

CLAassistant commented Jul 17, 2024 •

edited

Loading

github-actions bot commented Jul 17, 2024 •

edited

Loading

github-actions bot commented Jul 17, 2024 •

edited

Loading