Cellular: BC95 echo test fixes #6291
Conversation
|
@jarvte please review this |
|
Are all fixes related thus lead to one commit ? I can see addition here |
|
QUECTEL_BC95_CellularSIM addition is needed in order to get this module running past the SIM state checking. AT+CPIN? used in base class: AT_CellularSIM::get_sim_state is not supported AT command for this module. So when I took in use latest mbed-os-example-cellular and mbed-os, these were the changes I needed to do in order to get the echo test pass. The debug info I just found it to be useful while doing and testing these changes, that's why I added them at this point. |
|
@mirelachirica Thanks, my point was only about having it squashed into one commit (why are these in one commit - if all are related and fixing the same thing OK, otherwise use separate commits ). |
|
It is true that three different issues are addressed in this one commit(socket send/recv, SIM state, APN lookup logic). But all three were necessary to get example app's echo test pass. Do I have to generate new pull request with separate commits or will I just keep this in mind for future pull requests? |
You can rebase/amend this commit here, should be straightforward to split them. |
Fixing get host by name and iterate send/recv socket operations.
mirelachirica
force-pushed
the
bc95_echotest_fixes
branch
from
1db6e19 to
be95f47
Compare
|
/morph build |
Build : SUCCESSBuild number : 1390 Triggering tests/morph test |
Exporter Build : SUCCESSBuild number : 1045 |
Test : SUCCESSBuild number : 1179 |
|
ARM internal ref: IOTCELL-603, IOTCELL-649, IOTCELL-665 |
| QUECTEL_BC95_CellularSIM(ATHandler &atHandler); | ||
| virtual ~QUECTEL_BC95_CellularSIM(); | ||
|
|
||
| public: //from CellularSIM |
There was a problem hiding this comment.
Funky. Is there a specific reason why public: is declared twice?
There was a problem hiding this comment.
Not really. It is copied from other modules where it is used for specifying in comments the different interfaces the class is implementing, but of course comment alone would be enough.
| using namespace mbed; | ||
|
|
||
| QUECTEL_BC95_CellularSIM::QUECTEL_BC95_CellularSIM(ATHandler &atHandler) : AT_CellularSIM(atHandler) | ||
| { |
There was a problem hiding this comment.
Is there a reason the constructor and destructor are empty?
There was a problem hiding this comment.
The "empty" constructor is needed for passing atHandler to base AT_CellularSIM.
But empty destructor is not needed.
Description
Issues fixed: getting host by name, socket send/recv iterations, SIM state retrieval. Additionaly, cellular APN lookup decision logic is fixed.
These issues were found when testing BC95 against https://github.com/ARMmbed/mbed-os-example-cellular.
Pull request type