Skip to content

Cellular: Add unsupported features for UBLOX targets #8371

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 30, 2018
Merged

Cellular: Add unsupported features for UBLOX targets #8371

merged 1 commit into from
Oct 30, 2018

Conversation

mudassar-ublox
Copy link
Contributor

@mudassar-ublox mudassar-ublox commented Oct 11, 2018 •
edited by cmonr
Loading

Description

Added unsupported features for different UBLOX targets.TRAGET_UBLOX_C027 and TARGET_UBLOX_C030_R410M does not support parameters with AT+CGSN command.
In u-blox AT Command Manual and R410M AT Command Manual Section 4.7.4: AT+CGSN=snt, The snt parameter is not supported.

Pull request type

[ ] Fix
[ ] Refactor
[x] Target update
[ ] Functionality change
[ ] Breaking change

@cmonr cmonr requested review from a team October 11, 2018 15:18
@0xc0170 0xc0170 changed the title Cellular: Added unsupported features for UBLOX targets Cellular: Add unsupported features for UBLOX targets Oct 11, 2018
@0xc0170
Copy link
Contributor

0xc0170 commented Oct 23, 2018

@ARMmbed/mbed-os-wan Please review

@cmonr
Copy link
Contributor

cmonr commented Oct 25, 2018

Updated description to indicate target-specific updates.

@ARMmbed/mbed-os-wan Thoughts? This should be a relatively quick review.

cmonr
cmonr reviewed Oct 25, 2018
View reviewed changes
features/cellular/framework/targets/UBLOX/AT/UBLOX_AT.cpp
@@ -22,8 +22,16 @@
using namespace mbed;
using namespace events;

static const AT_CellularBase::SupportedFeature unsupported_features[] = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not too familiar with the cellular codebase, but why are these indicated as unsupported features?

And is there a reason they're now being enabled and/or supported?

Copy link
Contributor

Choose a reason for hiding this comment

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

@ARMmbed/team-ublox

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In cellularInformation test case, test_information_interface, it is reading serial number with different parameters of AT+CGSN command and some Ublox modem does not support parameters with AT+CGSN command, resulting failure of test. So thats why added it as unsupported feature.

AriParkkila
AriParkkila approved these changes Oct 25, 2018
View reviewed changes
features/cellular/framework/targets/UBLOX/AT/UBLOX_AT.cpp
@@ -22,8 +22,16 @@
using namespace mbed;
using namespace events;

static const AT_CellularBase::SupportedFeature unsupported_features[] = {
Copy link

Choose a reason for hiding this comment

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

#ifdef TARGET_UBLOX_C030_R410M

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you want me to add #ifdef over that static const AT_CellularBase::SupportedFeature unsupported_features ?

Copy link

Choose a reason for hiding this comment

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

To avoid warning: unused variable.

Copy link
Contributor

@0xc0170 0xc0170 Oct 25, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I would say yes, otherwise for other targets, this might be in the final image? plus the warning

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. Please review now.

features/cellular/framework/targets/UBLOX/PPP/UBLOX_PPP.cpp
@@ -22,8 +22,16 @@
using namespace mbed;
using namespace events;

static const AT_CellularBase::SupportedFeature unsupported_features[] = {
Copy link

Choose a reason for hiding this comment

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

#ifdef TARGET_UBLOX_C027

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 25, 2018

AriParkkila approved these changes

Should be request changes rather as the 2 comments above ?

@mudassar-ublox mudassar-ublox force-pushed the ublox-cellular-unsupported-features branch from bbb7efe to 0b2d508 Compare October 26, 2018 05:08
@0xc0170
Copy link
Contributor

0xc0170 commented Oct 26, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Oct 26, 2018

Build : FAILURE

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

@cmonr
Copy link
Contributor

cmonr commented Oct 29, 2018

14:11:34 abort: HTTP Error 502: Bad Gateway
14:11:34 ERROR : unable to clone the repo mbed-os-example-tls-hashing

Relaunching.
/morph build

@mbed-ci
Copy link

mbed-ci commented Oct 29, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci
Copy link

mbed-ci commented Oct 29, 2018

Exporter Build : SUCCESS

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

@cmonr
Copy link
Contributor

cmonr commented Oct 29, 2018

/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Oct 29, 2018

Test : FAILURE

Build number : 3282
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/8371/3282

@cmonr
Copy link
Contributor

cmonr commented Oct 29, 2018

Funky. Most of those test failures were due to TCP timeouts. Retesting.
/morph test

@mbed-ci
Copy link

mbed-ci commented Oct 30, 2018

Test : FAILURE

Build number : 3286
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/8371/3286

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 30, 2018

/morph test

@mbed-ci
Copy link

mbed-ci commented Oct 30, 2018

Test : SUCCESS

Build number : 3288
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/8371/3288

@cmonr cmonr merged commit 66968c5 into ARMmbed:master Oct 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmonr cmonr cmonr left review comments

@AriParkkila AriParkkila AriParkkila approved these changes

@0xc0170 0xc0170 0xc0170 approved these changes

@fahimalavi fahimalavi fahimalavi approved these changes

Assignees
No one assigned
Labels
release-version: 5.10.3
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mudassar-ublox @0xc0170 @cmonr @mbed-ci @fahimalavi @AriParkkila