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

BG96 operator name and access technology (IDFGH-3844) #5750

Closed
wants to merge 2 commits into from
Closed

BG96 operator name and access technology (IDFGH-3844) #5750

wants to merge 2 commits into from

Conversation

AshUK
Copy link
Contributor

@AshUK AshUK commented Aug 18, 2020

This PR adds the following:

  • Adds get operator functionality for BG96, currently the COPS command is only called at initialisation, if the modem does not yet have at network connection the operator name will be empty. Calling get operator will update the operator name value.

  • Adds the access technology used by the modem, this is required when the application logic is dependent on the access technology used e.g. NB IoT applications need to send messages via COAP and 2G uses MQTT.

  • Fixes a stack smashing issue on BG96

@CLAassistant
Copy link

CLAassistant commented Aug 18, 2020

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot changed the title Adds get operator, Adds access technology, BG96 Adds get operator, Adds access technology, BG96 (IDFGH-3844) Aug 18, 2020
@AshUK AshUK changed the title Adds get operator, Adds access technology, BG96 (IDFGH-3844) BG96 operator name and access technology (IDFGH-3844) Aug 18, 2020
@Alvin1Zhang
Copy link
Collaborator

Thanks for your contribution.

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 again for this useful update and for catching the stack issue!

I have left some minor comments to the changes.
Could you please rebase your changes to the latest master (this version is based on v4.1)?

@@ -193,9 +193,9 @@ static esp_err_t bg96_handle_cops(modem_dce_t *dce, const char *line)
size_t len = strlen(line);
char *line_copy = malloc(len + 1);
strcpy(line_copy, line);
/* +COPS: <mode>[, <format>[, <oper>]] */
/* +COPS: <mode>[, <format>[, <oper>][, <Act>]] */
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick

 /* +COPS: <mode>[, <format>[, <oper>[, <Act>]]] */

Copy link
Collaborator

Choose a reason for hiding this comment

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

Done

@@ -210,6 +210,11 @@ static esp_err_t bg96_handle_cops(modem_dce_t *dce, const char *line)
err = ESP_OK;
}
}
if (i >= 4) {
char act_str[3] = {0};
int len = snprintf(act_str, 3, "%s", p[3]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: This will produce defined but not used warning. Do we want to check if the len is correct?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Done

@@ -467,8 +474,6 @@ modem_dce_t *bg96_init(modem_dte_t *dte)
DCE_CHECK(bg96_get_imei_number(bg96_dce) == ESP_OK, "get imei failed", err_io);
/* Get IMSI number */
DCE_CHECK(bg96_get_imsi_number(bg96_dce) == ESP_OK, "get imsi failed", err_io);
/* Get operator name */
DCE_CHECK(bg96_get_operator_name(bg96_dce) == ESP_OK, "get operator name failed", err_io);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Removing get_operator_name() from init might cause an inconsistency in idf examples. I would suggest adding

    ESP_ERROR_CHECK(dce->get_operator_name(dce));

to the pppos_client_main.c after the specific initialization section.

This may require applying the same changes to other devices (SIM800, ..) as well

Copy link
Collaborator

Choose a reason for hiding this comment

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

fallback

espressif-bot pushed a commit that referenced this pull request Jan 15, 2021
Merges #5750
Signed-off-by: Liu Han <[email protected]>
@AshUK
Copy link
Contributor Author

AshUK commented Feb 12, 2021

Closing this as it has been now been added to v4.3-beta1, thank you to those that contributed.

@AshUK AshUK closed this Feb 12, 2021
espressif-bot pushed a commit that referenced this pull request Mar 3, 2022
Merges #5750
Signed-off-by: Liu Han <[email protected]>
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.

6 participants