Fix CGI for Quectel modems

[zfields]

submission notes

**Important:** Please sanitize/remove any confidential info like usernames, passwords, org names, product names/ids, access tokens, client ids/secrets, or anything else you don't wish to share.

Please Read and Sign the Contributor License Agreement ([Info here](https://github.com/spark/firmware/blob/develop/CONTRIBUTING.md)).

You may also delete this submission notes header if you'd like. Thank you for contributing!

Problem

Quectel modems do not correctly report CGI data.

quectel_ncp_client.cpp appears to have been copied before CGI data was finished.

This results in not setting the flags TWO_DIGIT_MNC flag value, and the lac and ci are reported from uninitialized values.

Solution

Copy the latest Gen3 implementation from sara_ncp_client.cpp.

Steps to Test

Create a sample application on new b5som hardware

Example App

void setup() {
  Particle.publishVitals(15);
}

void loop() {

}

References

ch46950

---

Completeness

• User is totes amazing for contributing!
• Contributor has signed CLA (Info here)
• Problem and Solution clearly stated
• Run unit/integration/application tests on device
• Added documentation
• Added to CHANGELOG.md after merging (add links to docs and issues)

[technobly]

Great job solving this so fast!

[zfields]

Codacy, does not like the scanf formatting. However, this same formatting is used by all our parser implementations. Fixing this one, or all of them, introduces unnecessary risk to a low risk, functional PR.

I will make a CH story and annotate the code with //TODO: statements that reference the CH ticket.

//TODO: Use correct data types to parse URC
//[ch46994](https://app.clubhouse.io/particle/story/46994/fix-scanf-input-formatting-for-at-creg-et-al)
//Warning: %x in format string (no. 3) requires 'unsigned int *' but the argument type is 'signed int *'.

ch46994

[technobly]

I’m also kind of wondering why the extra %*x is there now as well. 6 Params and 5 params instead of the spec’d 5 and 4.

…

On Feb 10, 2020, at 11:19 PM, keer ***@***.***> wrote:  @keeramis approved this pull request. In hal/src/b5som/network/quectel_ncp_client.cpp: > return SYSTEM_ERROR_NONE; }, this)); CHECK(parser_.addUrcHandler("+CGREG", [](AtResponseReader* reader, const char* prefix, void* data) -> int { const auto self = (QuectelNcpClient*)data; - int val[2]; - int r = CHECK_PARSER_URC(reader->scanf("+CGREG: %d,%d", &val[0], &val[1])); - CHECK_TRUE(r >= 1, SYSTEM_ERROR_UNKNOWN); + int val[4] = {-1,-1,-1,-1}; + char atResponse[64] = {0}; + // Take a copy of AT response for multi-pass scanning + CHECK_PARSER_URC(reader->readLine(atResponse, sizeof(atResponse))); + // Parse response ignoring mode (replicate URC response) + int r = ::sscanf(atResponse, "+CGREG: %*d,%d,\"%x\",\"%x\",%d,\"%*x\"", &val[0], &val[1], &val[2], &val[3]); + // Reparse URC as direct response CGREG AT command gives responses like this: If we query, the response format is this - we will have 5 values in the response string - +CGREG: n,stat[,lac,ci[,Act]] If it's a URC, the response format is - we will have 4 values in the response string - +CGREG: stat[,lac,ci[,Act]] In hal/src/b5som/network/quectel_ncp_client.cpp: > return SYSTEM_ERROR_NONE; }, this)); CHECK(parser_.addUrcHandler("+CGREG", [](AtResponseReader* reader, const char* prefix, void* data) -> int { const auto self = (QuectelNcpClient*)data; - int val[2]; - int r = CHECK_PARSER_URC(reader->scanf("+CGREG: %d,%d", &val[0], &val[1])); - CHECK_TRUE(r >= 1, SYSTEM_ERROR_UNKNOWN); + int val[4] = {-1,-1,-1,-1}; + char atResponse[64] = {0}; + // Take a copy of AT response for multi-pass scanning + CHECK_PARSER_URC(reader->readLine(atResponse, sizeof(atResponse))); + // Parse response ignoring mode (replicate URC response) + int r = ::sscanf(atResponse, "+CGREG: %*d,%d,\"%x\",\"%x\",%d,\"%*x\"", &val[0], &val[1], &val[2], &val[3]); + // Reparse URC as direct response + if (0 >= r) { + r = CHECK_PARSER_URC( + ::sscanf(atResponse, "+CGREG: %d,\"%x\",\"%x\",%d,\"%*x\"", &val[0], &val[1], &val[2], &val[3])); Nit: Why 5 parameters here, while this line (https://github.com/particle-iot/device-os/blob/82486ffdf7811904faa3dbc1ecad2e52377ecc49/hal/src/b5som/network/quectel_ncp_client.cpp#L223) has only 4 ? — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub, or unsubscribe.

[avtolstoy]

I agree with the points raised by other reviewers.

This code was copied over from sara_ncp_client.cpp and for ublox modems the responses may contain additional parameters like <rac> One byte routing area in hexadecimal format and others. These handlers also take care of both the read responses and URCs, so should take care of any format expected due to CH28408, however the format is still different between the modems, so let's make sure we tailor the parsing to Quectel AT commands manual.