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

Cellular: Add AT+CCID and AT+CGSN #7217

Merged
merged 2 commits into from
Jun 19, 2018
Merged

Conversation

AriParkkila
Copy link

Description

Added new methods in cellular API:

  • CellularInformation::get_serial_number to read SN and IMEI of modem with AT+CGSN
  • CellularSIM::get_iccid to read ICCID of SIM card with AT+CCID

Fixes Arm internal ref IOTCELL-1091.

Doxygen and return types of the CellularInformation class were fixed at the same.

Usage example:

CellularInformation *info = _cellularDevice->open_information(_serial);
char buf[100];
if (info->get_serial_number(buf, sizeof(buf), CellularInformation::SN) == 0) {
    tr_info("SN: %s", buf);
}
_cellularDevice->close_information();

Pull request type

[ ] Fix
[ ] Refactor
[ ] New target
[X] Feature
[ ] Breaking change

@AriParkkila
Copy link
Author

@mirelachirica @jarvte please review

AnttiKauppila
AnttiKauppila previously approved these changes Jun 14, 2018
@@ -21,7 +21,8 @@
#include <stddef.h>
#include "nsapi_types.h"

namespace mbed {
namespace mbed
{
Copy link
Contributor

@0xc0170 0xc0170 Jun 14, 2018

Choose a reason for hiding this comment

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

as it was - it was good ( see astyle in the travis ). It shows more style issues

Style changes, own commit please

Copy link
Author

Choose a reason for hiding this comment

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

This follows Mbed OS coding guidelines, see astyle in https://os.mbed.com/teams/SDK-Development/wiki/mbed-sdk-coding-style

Copy link
Contributor

@0xc0170 0xc0170 Jun 14, 2018

Choose a reason for hiding this comment

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

That is the old one. Please do not use it. Anyway, from there "Braces - K&R (see the exception 1 TBS below)" . this { should be attached.

Use https://os.mbed.com/docs/latest/reference/style.html (points to astylerc file that is in our codebase) . Travis runs this and can show you how this breaks the style defined there.

Taken from Travis:

-namespace mbed
-{
+namespace mbed {
 
 /**
  *  Class CellularInformation
  *
  *  An abstract interface that provides information about cellular device.
  */
-class CellularInformation
-{
+class CellularInformation {

@@ -18,12 +18,16 @@
#ifndef CELLULAR_SIM_H_
#define CELLULAR_SIM_H_

#include <stddef.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

how does this relates to the new API addition?

Copy link
Author

Choose a reason for hiding this comment

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

That's for size_t definition.

jarvte
jarvte previously approved these changes Jun 14, 2018
* @param buf_size max length of manufacturer identification is 2048 characters
* @return on success read character count, on failure negative error code
* @return zero on success, on failure negative error code
Copy link
Contributor

@mirelachirica mirelachirica Jun 14, 2018

Choose a reason for hiding this comment

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

Internal ticket IOTCELL-1097 to unify comments about return values on APIs returning nsapi_error_t.
Suggest: "NSAPI_ERROR_OK on success, or following error codes: -list of possible error codes-"

@AriParkkila
Copy link
Author

This is working with UBLOX_C027, but I noticed that other cellular targets need more work.

@AriParkkila
Copy link
Author

@mirelachirica @jarvte please re-review target specific adaptation

@AriParkkila
Copy link
Author

@0xc0170 this should be now good for merge, I think.

@0xc0170 0xc0170 changed the title Cellular: Added AT+CCID and AT+CGSN Cellular: Add AT+CCID and AT+CGSN Jun 18, 2018
@0xc0170
Copy link
Contributor

0xc0170 commented Jun 18, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Jun 18, 2018

Build : SUCCESS

Build number : 2368
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7217/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Jun 18, 2018

@mbed-ci
Copy link

mbed-ci commented Jun 18, 2018

@cmonr
Copy link
Contributor

cmonr commented Jun 18, 2018

/morph uvisor-test

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 19, 2018

@AnttiKauppila @jarvte Can you rereview please?

@cmonr cmonr merged commit fcfe6e1 into ARMmbed:master Jun 19, 2018
@AriParkkila AriParkkila deleted the cellular-info-sim branch September 10, 2018 08:26
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.

None yet

7 participants