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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions hal/inc/hal_platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -305,4 +305,8 @@
#define HAL_PLATFORM_PACKET_BUFFER_FLOW_CONTROL_THRESHOLD (0)
#endif // HAL_PLATFORM_PACKET_BUFFER_FLOW_CONTROL_THRESHOLD

#ifndef HAL_PLATFORM_MUXER_MAY_NEED_DELAY_IN_TX
#define HAL_PLATFORM_MUXER_MAY_NEED_DELAY_IN_TX (0)
#endif //HAL_PLATFORM_MUXER_MAY_NEED_DELAY_IN_TX

#endif /* HAL_PLATFORM_H */
1 change: 1 addition & 0 deletions hal/network/ncp/cellular/cellular_ncp_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ class CellularNcpClient: public NcpClient {
virtual int getImei(char* buf, size_t size) = 0;
virtual int getSignalQuality(CellularSignalQuality* qual) = 0;
virtual int setRegistrationTimeout(unsigned timeout) = 0;
virtual int getTxDelayInDataChannel() = 0;
};

inline CellularNcpClientConfig::CellularNcpClientConfig() :
Expand Down
4 changes: 4 additions & 0 deletions hal/src/b5som/network/quectel_ncp_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,10 @@ int QuectelNcpClient::setRegistrationTimeout(unsigned timeout) {
return 0;
}

int QuectelNcpClient::getTxDelayInDataChannel() {
return 0;
}

int QuectelNcpClient::checkParser() {
if (ncpState_ != NcpState::ON) {
return SYSTEM_ERROR_INVALID_STATE;
Expand Down
1 change: 1 addition & 0 deletions hal/src/b5som/network/quectel_ncp_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class QuectelNcpClient: public CellularNcpClient {
virtual int getImei(char* buf, size_t size) override;
virtual int getSignalQuality(CellularSignalQuality* qual) override;
virtual int setRegistrationTimeout(unsigned timeout) override;
virtual int getTxDelayInDataChannel() override;

auto getMuxer() {
return &muxer_;
Expand Down
1 change: 1 addition & 0 deletions hal/src/boron/hal_platform_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#define HAL_PLATFORM_NCP_AT (1)
#define HAL_PLATFORM_CELLULAR (1)
#define HAL_PLATFORM_SETUP_BUTTON_UX (1)
#define HAL_PLATFORM_MUXER_MAY_NEED_DELAY_IN_TX (1)

#if PLATFORM_ID == PLATFORM_BORON

Expand Down
45 changes: 37 additions & 8 deletions hal/src/boron/network/sara_ncp_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ const unsigned REGISTRATION_TIMEOUT = 10 * 60 * 1000;
using LacType = decltype(CellularGlobalIdentity::location_area_code);
using CidType = decltype(CellularGlobalIdentity::cell_id);

const system_tick_t UBLOX_NCP_R4_LARGE_PACKET_TIMEOUT_MS = 250;
const size_t UBLOX_NCP_R4_LARGE_PACKET_THRESHOLD_SIZE = 512;

} // anonymous

SaraNcpClient::SaraNcpClient() {
Expand Down Expand Up @@ -383,21 +386,40 @@ int SaraNcpClient::updateFirmware(InputStream* file, size_t size) {
return SYSTEM_ERROR_NOT_SUPPORTED;
}

/*
* This is a callback that writes data into muxer channel 2 (data PPP channel)
* Whenever we encounter a large packet, we enforce a certain number of ms to pass before
* transmitting anything else on this channel. After we send large packet, we drop messages(bytes)
* for a certain amount of time defined by UBLOX_NCP_R4_LARGE_PACKET_TIMEOUT_MS
*/
int SaraNcpClient::dataChannelWrite(int id, const uint8_t* data, size_t size) {
if (fwVersion_ <= UBLOX_NCP_R4_APP_FW_VERSION_NO_HW_FLOW_CONTROL_MAX) {
if (lastLargePacket_ && (HAL_Timer_Get_Milli_Seconds() - lastLargePacket_) < UBLOX_NCP_R4_LARGE_PACKET_TIMEOUT_MS) {
// Drop
LOG_DEBUG(WARN, "Dropping");
return 0;
}
}

int err = muxer_.writeChannel(UBLOX_NCP_PPP_CHANNEL, data, size);
if (err == gsm0710::GSM0710_ERROR_FLOW_CONTROL) {
// Not an error
LOG_DEBUG(WARN, "Remote side flow control");
err = 0;
}

if (fwVersion_ <= UBLOX_NCP_R4_APP_FW_VERSION_NO_HW_FLOW_CONTROL_MAX) {
if (size >= UBLOX_NCP_R4_LARGE_PACKET_THRESHOLD_SIZE) {
lastLargePacket_ = HAL_Timer_Get_Milli_Seconds();
} else {
lastLargePacket_ = 0;
}
}
if (err) {
// Make sure we are going into an error state if muxer for some reason fails
// to write into the data channel.
LOG(ERROR, "Failed to write into data channel %d", err);
disable();
}

return err;
}

Expand Down Expand Up @@ -465,6 +487,13 @@ int SaraNcpClient::getImei(char* buf, size_t size) {
return n;
}

int SaraNcpClient::getTxDelayInDataChannel() {
if (ncpId() == PLATFORM_NCP_SARA_R410 && fwVersion_ <= UBLOX_NCP_R4_APP_FW_VERSION_NO_HW_FLOW_CONTROL_MAX) {
return UBLOX_NCP_R4_LARGE_PACKET_TIMEOUT_MS * 2;
}
return 0;
}

int SaraNcpClient::queryAndParseAtCops(CellularSignalQuality* qual) {
int act;
char mobileCountryCode[4] = {0};
Expand Down Expand Up @@ -932,13 +961,13 @@ int SaraNcpClient::initReady() {
// Change the baudrate to 921600
CHECK(changeBaudRate(UBLOX_NCP_RUNTIME_SERIAL_BAUDRATE_U2));
} else {
int fwVersion = getAppFirmwareVersion();
if (fwVersion > 0) {
fwVersion_ = getAppFirmwareVersion();
if (fwVersion_ > 0) {
// L0.0.00.00.05.06,A.02.00 has a memory issue
memoryIssuePresent_ = (fwVersion == UBLOX_NCP_R4_APP_FW_VERSION_MEMORY_LEAK_ISSUE);
memoryIssuePresent_ = (fwVersion_ == UBLOX_NCP_R4_APP_FW_VERSION_MEMORY_LEAK_ISSUE);
// There is a set of other revisions which do not have hardware flow control
if (!(fwVersion >= UBLOX_NCP_R4_APP_FW_VERSION_NO_HW_FLOW_CONTROL_MIN &&
fwVersion <= UBLOX_NCP_R4_APP_FW_VERSION_NO_HW_FLOW_CONTROL_MAX)) {
if (!(fwVersion_ >= UBLOX_NCP_R4_APP_FW_VERSION_NO_HW_FLOW_CONTROL_MIN &&
fwVersion_ <= UBLOX_NCP_R4_APP_FW_VERSION_NO_HW_FLOW_CONTROL_MAX)) {
// Change the baudrate to 460800
// NOTE: ignoring AT errors just in case to accommodate for some revisions
// potentially not supporting anything other than 115200
Expand Down Expand Up @@ -1043,7 +1072,7 @@ int SaraNcpClient::initReady() {
}

// Send AT+CMUX and initialize multiplexer
r = CHECK_PARSER(parser_.execCommand("AT+CMUX=0,0,,1509,,,,,"));
r = CHECK_PARSER(parser_.execCommand("AT+CMUX=0,0,,%u,,,,,", UBLOX_NCP_MAX_MUXER_FRAME_SIZE));
CHECK_TRUE(r == AtResponse::OK, SYSTEM_ERROR_AT_NOT_OK);

// Initialize muxer
Expand Down
4 changes: 4 additions & 0 deletions hal/src/boron/network/sara_ncp_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class SaraNcpClient: public CellularNcpClient {
virtual int getImei(char* buf, size_t size) override;
virtual int getSignalQuality(CellularSignalQuality* qual) override;
virtual int setRegistrationTimeout(unsigned timeout) override;
virtual int getTxDelayInDataChannel() override;

private:
AtParser parser_;
Expand Down Expand Up @@ -95,10 +96,13 @@ class SaraNcpClient: public CellularNcpClient {
system_tick_t regCheckTime_;
system_tick_t registeredTime_;
system_tick_t powerOnTime_;
unsigned int fwVersion_ = 0;
bool memoryIssuePresent_ = false;
unsigned registrationTimeout_;
volatile bool inFlowControl_ = false;

system_tick_t lastLargePacket_ = 0;

int queryAndParseAtCops(CellularSignalQuality* qual);
int initParser(Stream* stream);
int checkParser();
Expand Down
17 changes: 17 additions & 0 deletions system/src/system_cloud_internal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@
#include "system_network_internal.h"
#include "str_util.h"
#include "scope_guard.h"
#if HAL_PLATFORM_MUXER_MAY_NEED_DELAY_IN_TX
#include "network/ncp/cellular/ncp.h"
#include "network/ncp/cellular/cellular_ncp_client.h"
#endif // HAL_PLATFORM_MUXER_MAY_NEED_DELAY_IN_TX

#include <stdio.h>
#include <stdint.h>
Expand Down Expand Up @@ -1053,6 +1057,19 @@ int Spark_Handshake(bool presence_announce)
LOG(INFO,"Starting handshake: presense_announce=%d", presence_announce);
bool session_resumed = false;
int err = spark_protocol_handshake(sp);

#if HAL_PLATFORM_MUXER_MAY_NEED_DELAY_IN_TX
// XXX: Adding a delay only for platforms Boron and BSoM, because older cell versions of
// Boron R410M modems crash when handshake messages are sent without gap.
// We have a workaround for this issue in the NCP client, so the modem should no longer crash
// in any case, but we want to avoid dropping a set of publishes which are generated below,
// hence a delay here as a workaround.
auto timeout = particle::cellularNetworkManager()->ncpClient()->getTxDelayInDataChannel();
if (timeout > 0) {
HAL_Delay_Milliseconds(timeout);
}
#endif // HAL_PLATFORM_MUXER_MAY_NEED_DELAY_IN_TX

if (err == particle::protocol::SESSION_RESUMED) {
session_resumed = true;
} else if (err != 0) {
Expand Down