Skip to content

add ESP8266 interface suspend and resume APIs #12987

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

Closed

Conversation

soleilplanet
Copy link
Contributor

Summary of changes

Add and expose ESP8266 interface suspend and resume APIs to allow user applications to perform power management control with deep sleep

Impact of changes

New APIs exposed to enable/disable traffic over ESP8266 input or output interface

Migration actions required

N/A

Documentation

Need to add the API descriptions of the interface_suspend() and interface_resume()


Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[X] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[X] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@kjbracey-arm

@mergify mergify bot added the needs: work label May 18, 2020
@ciarmcom ciarmcom requested review from kjbracey and a team May 18, 2020 07:00
@ciarmcom
Copy link
Member

@soleilplanet, thank you for your changes.
@kjbracey-arm @ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-maintainers please review.

@0xc0170
Copy link
Contributor

0xc0170 commented May 18, 2020

@soleilplanet please fix the astyle errors

*
* @param direction The direction going to be suspended
*/
virtual void interface_suspend(uint16_t direction);
Copy link
Contributor

Choose a reason for hiding this comment

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

suspend/resume wouldn't be sufficient (no need for interface_) ? I dont see it defined anywhere in Mbed OS codebase as it is here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well I put the interface_ here because I don't want to confuse the users. The API is not to suspend/resume the ESP8266 itself, but only the traffic.
But on second thought, the class itself is ESP8266Interface, so perhaps it wouldn't confuse users. How do you think?

@adbridge
Copy link
Contributor

@0xc0170 can you answer the questions please? @ARMmbed/mbed-os-ipcore could someone review this ?

@michalpasztamobica
Copy link
Contributor

@soleilplanet , with this PR we enabled deep sleep whenever ESP gets disconnected. I think this API extension is not needed.

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 17, 2020

@soleilplanet , with this PR we enabled deep sleep whenever ESP gets disconnected. I think this API extension is not needed.

Based on this, I'll close the PR. If it's not correct, please let us know or just reopen the PR with a reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants