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

Boron LTE-M1 radio unresponsive when sending large data continuously #2100

Merged
merged 4 commits into from
May 14, 2020

Conversation

keeramis
Copy link
Contributor

@keeramis keeramis commented May 13, 2020

Problem

Boron LTE-M1 cell radio becomes unresponsive (by getting stuck at an AT command) after "large" data is sent continuously over muxer data channel

Solution

Whenever we encounter a large packet, we enforce a certain number of milliseconds (set to 250ms) to pass before transmitting anything else on this channel. After we send a "large" packet (of size 512 bytes), we drop messages(bytes) for a certain amount of time defined by largePacketTimeoutMs_ (250ms)

Steps to Test

Example App - 1 - Basic connectivity to cloud since proactively sending describe messages to cloud without waiting for a request from cloud

void setup() {
}

void loop() {
}

Example App - 2 - Sending large data packets to cloud

void setup() {
}

void loop() {
    delay(1000);
    Particle.publish("temperature", "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", PRIVATE);
}

References

ch53040


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)

@@ -108,4 +108,8 @@
#define PRODUCT_SERIES "electron"
#endif

#if PLATFORM_ID == PLATFORM_BSOM
#define HAL_PLATFORM_MUXER_MAY_NEED_DELAY_IN_TX (1)
#endif // PLATFORM_ID == PLATFORM_BSOM
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@avtolstoy Please verify if this is the correct place to set for PLATFORM_BSOM

@@ -108,4 +108,8 @@
#define PRODUCT_SERIES "electron"
#endif

#if PLATFORM_ID == PLATFORM_BSOM
Copy link
Member

Choose a reason for hiding this comment

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

This is the wrong file to define this.

bool memoryIssuePresent_ = false;
unsigned registrationTimeout_;
volatile bool inFlowControl_ = false;

const system_tick_t largePacketTimeoutMs_ = 250;
Copy link
Member

Choose a reason for hiding this comment

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

Let's probably move this to the source file into the anonymous namespace constants.

// Boron R410M modems crash when handshake messages are sent without gap
#if HAL_PLATFORM_MUXER_MAY_NEED_DELAY_IN_TX
if (platform_current_ncp_identifier() == PLATFORM_NCP_SARA_R410) {
HAL_Delay_Milliseconds(1000);
Copy link
Member

Choose a reason for hiding this comment

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

Do we perhaps want to query the firmware version here as well? I think it should be alright to directly query SaraNcpClient here and perhaps make the timeout come from there as well to avoid using a magic value. So something along the lines of:

#if HAL_PLATFORM_MUXER_MAY_NEED_DELAY_IN_TX
auto timeout = cellularNetworkManager()->ncpClient()->mayNeedTxDelay(); // don't judge the name :)
if (timeout > 0) {
    HAL_Delay_Milliseconds(timeout);
}
#endif // HAL_PLATFORM_MUXER_MAY_NEED_DELAY_IN_TX

@@ -18,6 +18,7 @@
#define HAL_PLATFORM_FUELGAUGE_MAX17043_I2C (HAL_I2C_INTERFACE2)
#define HAL_PLATFORM_RADIO_ANTENNA_INTERNAL (1)
#define HAL_PLATFORM_RADIO_ANTENNA_EXTERNAL (1)
#define HAL_PLATFORM_MUXER_MAY_NEED_DELAY_IN_TX (1)
Copy link
Member

Choose a reason for hiding this comment

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

This also needs to be defined for B SoM: the #else condition here.

@avtolstoy avtolstoy force-pushed the ch53040/fix_boron_rapid_cyan branch from 8f42924 to 39bad7c Compare May 13, 2020 18:36
@avtolstoy avtolstoy force-pushed the ch53040/fix_boron_rapid_cyan branch from 39bad7c to bc2ab95 Compare May 13, 2020 18:37
@avtolstoy avtolstoy added ready to merge PR has been reviewed and tested and removed needs review labels May 14, 2020
@avtolstoy avtolstoy added this to the 2.0.0 milestone May 14, 2020
@avtolstoy avtolstoy merged commit b0aae91 into develop May 14, 2020
@avtolstoy avtolstoy deleted the ch53040/fix_boron_rapid_cyan branch May 14, 2020 13:41
@keeramis keeramis mentioned this pull request May 18, 2020
6 tasks
@avtolstoy avtolstoy mentioned this pull request May 18, 2020
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ready to merge PR has been reviewed and tested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants