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.

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

esp_modem_dce_service: Make esp_modem_dce_handle_* functions static (IDFGH-5542) #7266

Closed

Conversation

AxelLin
Copy link
Contributor

@AxelLin AxelLin commented Jul 13, 2021

These functions are unlikely to be reused, make them static.
Take esp_modem_dce_handle_cbc as an example, it's internal implementation
of esp_modem_dce_get_battery_status, expose esp_modem_dce_get_battery_status
function is enough.

Signed-off-by: Axel Lin axel.lin@gmail.com

@espressif-bot espressif-bot added the Status: Opened Issue is new label Jul 13, 2021
@github-actions github-actions bot changed the title esp_modem_dce_service: Make esp_modem_dce_handle_* functions static esp_modem_dce_service: Make esp_modem_dce_handle_* functions static (IDFGH-5542) Jul 13, 2021
Copy link
Collaborator

@david-cermak david-cermak left a comment

Choose a reason for hiding this comment

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

Thanks for this PR. Yes, I think it's a good idea to make the functions for handling replies static.

Could you please just move the doxy comments next to the definitions. Even if it's not a public API, i think it's a good idea to keep the documentation, at least the first @brief line.

These functions are unlikely to be reused, make them static.
Take esp_modem_dce_handle_cbc as an example, it's internal implementation
of esp_modem_dce_get_battery_status, expose esp_modem_dce_get_battery_status
function is enough.

Signed-off-by: Axel Lin <axel.lin@gmail.com>
@AxelLin AxelLin force-pushed the esp_modem_dce_service_improve branch from 50b58d2 to fcc96a3 Compare July 23, 2021 23:41
Copy link
Collaborator

@david-cermak david-cermak left a comment

Choose a reason for hiding this comment

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

Thanks for the quick update. LGTM!

@espressif-bot espressif-bot added Status: In Progress Work is in progress Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: Opened Issue is new Status: In Progress Work is in progress labels Jul 26, 2021
espressif-bot pushed a commit that referenced this pull request Jul 29, 2021
…s static

These functions are unlikely to be reused, make them static.
Take esp_modem_dce_handle_cbc as an example, it's internal implementation
of esp_modem_dce_get_battery_status, expose esp_modem_dce_get_battery_status
function is enough.

Signed-off-by: Axel Lin <axel.lin@gmail.com>

Merges #7266
@Alvin1Zhang
Copy link
Collaborator

Thanks for your contribution, changes have been merged with d6248ea.

@AxelLin AxelLin deleted the esp_modem_dce_service_improve branch August 12, 2021 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants