Skip to content

Conversation

oscarh
Copy link
Contributor

@oscarh oscarh commented Jan 15, 2019

Description

The CellularContext does have the member nsapi_connection_status_t _connect_status, which can simple be returned to support the get_connection_status() API.

The second part is UBLOX_AT specific, where the callback never is called, as the network_callback helper method actually wants to set the status as well, and doesn't do anything if the status isn't changed.

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

Add override for the virtual method get_connection_status() inherited
from NetworkInterface. The method in the base class returns
NSAPI_STATUS_ERROR_UNSUPPORTED. The CellularContext has the member
_connection_status, which means that we could return this.
The helper method call_network_cb, actually does a lot more than calling
the callback. The method has a check that the network status supplied
for the callback is different compared to the internal one. It also sets
the class member if it is changed. This is a bit surprising, given the
name of the method. It also means that it doesn't work in this call, as
the member is already set.
@ciarmcom ciarmcom requested a review from a team January 15, 2019 12:13
@ciarmcom
Copy link
Member

@oscarh, thank you for your changes.
@ARMmbed/mbed-os-wan @ARMmbed/mbed-os-maintainers please review.

@ciarmcom ciarmcom requested a review from a team January 15, 2019 12:13
@0xc0170 0xc0170 requested a review from a team January 16, 2019 08:22
@0xc0170
Copy link
Contributor

0xc0170 commented Jan 21, 2019

@ARMmbed/team-ublox Can you please review?

@fahimalavi
Copy link
Contributor

OK in review

@fahimalavi
Copy link
Contributor

Looks OK

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 22, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Jan 22, 2019

Test run: FAILED

Summary: 1 of 1 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_unittests

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 22, 2019

Can you review unittest failures? Open build artifacts above in the test report

[linux] CMakeFiles/features-cellular-framework-AT-at_cellulardevice.dir/stubs/AT_CellularContext_stub.cpp.o:(.rodata._ZTVN4mbed18AT_CellularContextE[_ZTVN4mbed18AT_CellularContextE]+0x88): undefined reference to mbed::AT_CellularContext::get_connection_status() const'`

@oscarh
Copy link
Contributor Author

oscarh commented Jan 22, 2019 via email

@bqam-ublox
Copy link
Contributor

Hi @oscarh , it seems like you need to add a stub function in AT_CellularContext_stub.cpp file to resolve the unit test failure.

@cmonr
Copy link
Contributor

cmonr commented Jan 30, 2019

@oscarh Any update?

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 4, 2019

CI restarted

@mbed-ci
Copy link

mbed-ci commented Feb 4, 2019

Test run: SUCCESS

Summary: 12 of 12 test jobs passed
Build number : 2
Build artifacts

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.

9 participants