From feee2a234e3b3976e834f90824a15b114c6f19ef Mon Sep 17 00:00:00 2001 From: Ari Parkkila Date: Wed, 4 Dec 2019 03:43:01 -0800 Subject: [PATCH] Cellular: Fix Non-IP of Quectel/BG96 modem driver Add missing license headers. Remove semaphores to allow async operation. Change hex format to allow binary payload. Limit payload size to 100 bytes. Fix missing plus char at +QCFGEXT. --- features/cellular/framework/AT/ATHandler.cpp | 2 +- features/cellular/framework/AT/ATHandler.h | 2 +- .../BG96/QUECTEL_BG96_CellularContext.cpp | 9 ++- .../BG96/QUECTEL_BG96_CellularContext.h | 1 - .../BG96/QUECTEL_BG96_ControlPlane_netif.cpp | 62 ++++++++++++++----- 5 files changed, 53 insertions(+), 23 deletions(-) diff --git a/features/cellular/framework/AT/ATHandler.cpp b/features/cellular/framework/AT/ATHandler.cpp index 3e0ee67207..f3c311bcb7 100644 --- a/features/cellular/framework/AT/ATHandler.cpp +++ b/features/cellular/framework/AT/ATHandler.cpp @@ -1637,7 +1637,7 @@ void ATHandler::set_send_delay(uint16_t send_delay) _at_send_delay = send_delay; } -void ATHandler::write_hex_string(char *str, size_t size) +void ATHandler::write_hex_string(const char *str, size_t size) { // do common checks before sending subparameter if (check_cmd_send() == false) { diff --git a/features/cellular/framework/AT/ATHandler.h b/features/cellular/framework/AT/ATHandler.h index 2ab937eb6a..b41aae6e53 100644 --- a/features/cellular/framework/AT/ATHandler.h +++ b/features/cellular/framework/AT/ATHandler.h @@ -433,7 +433,7 @@ public: * @param str input buffer to be converted to hex ascii * @param size of the input param str */ - void write_hex_string(char *str, size_t size); + void write_hex_string(const char *str, size_t size); /** Reads as string and converts result to integer. Supports only non-negative integers. * diff --git a/features/cellular/framework/targets/QUECTEL/BG96/QUECTEL_BG96_CellularContext.cpp b/features/cellular/framework/targets/QUECTEL/BG96/QUECTEL_BG96_CellularContext.cpp index a580f0a3f6..542d73b708 100644 --- a/features/cellular/framework/targets/QUECTEL/BG96/QUECTEL_BG96_CellularContext.cpp +++ b/features/cellular/framework/targets/QUECTEL/BG96/QUECTEL_BG96_CellularContext.cpp @@ -84,9 +84,13 @@ nsapi_error_t QUECTEL_BG96_CellularContext::activate_non_ip_context() // Open the NIDD connection nsapi_size_or_error_t ret = _at.at_cmd_discard("+QCFGEXT", "=\"nipd\",1"); - if (ret == NSAPI_ERROR_OK) { - _semaphore.try_acquire_for(NIDD_OPEN_URC_TIMEOUT); + _at.lock(); + _at.set_at_timeout(NIDD_OPEN_URC_TIMEOUT); + _at.resp_start("+QIND:"); + urc_nidd(); + _at.restore_at_timeout(); + _at.unlock(); if (_cid == -1) { return NSAPI_ERROR_NO_CONNECTION; } @@ -142,7 +146,6 @@ void QUECTEL_BG96_CellularContext::urc_nidd_open() } else { tr_error("NIDD connection open failed with error: %d", err); } - _semaphore.release(); } void QUECTEL_BG96_CellularContext::urc_nidd_close() diff --git a/features/cellular/framework/targets/QUECTEL/BG96/QUECTEL_BG96_CellularContext.h b/features/cellular/framework/targets/QUECTEL/BG96/QUECTEL_BG96_CellularContext.h index c3f1a5ec20..4e536c4090 100644 --- a/features/cellular/framework/targets/QUECTEL/BG96/QUECTEL_BG96_CellularContext.h +++ b/features/cellular/framework/targets/QUECTEL/BG96/QUECTEL_BG96_CellularContext.h @@ -37,7 +37,6 @@ protected: virtual void deactivate_non_ip_context(); virtual void deactivate_context(); virtual void activate_context(); - rtos::Semaphore _semaphore; private: void urc_nidd(); diff --git a/features/cellular/framework/targets/QUECTEL/BG96/QUECTEL_BG96_ControlPlane_netif.cpp b/features/cellular/framework/targets/QUECTEL/BG96/QUECTEL_BG96_ControlPlane_netif.cpp index 39ab44fe07..247cc55647 100644 --- a/features/cellular/framework/targets/QUECTEL/BG96/QUECTEL_BG96_ControlPlane_netif.cpp +++ b/features/cellular/framework/targets/QUECTEL/BG96/QUECTEL_BG96_ControlPlane_netif.cpp @@ -1,4 +1,25 @@ +/* + * Copyright (c) 2018, Arm Limited and affiliates. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "CellularUtil.h" #include "QUECTEL_BG96_ControlPlane_netif.h" +#include "CellularLog.h" + +using namespace mbed_cellular_util; namespace mbed { @@ -7,21 +28,26 @@ QUECTEL_BG96_ControlPlane_netif::QUECTEL_BG96_ControlPlane_netif(ATHandler &at, nsapi_size_or_error_t QUECTEL_BG96_ControlPlane_netif::send(const void *data, nsapi_size_t size) { - nsapi_size_or_error_t err = _at.at_cmd_discard("+QCFGEXT", "=\"nipds\",0,", "%s%d", data, size); - - if (err == NSAPI_ERROR_OK) { - return size; + if (size > 100) { // from BG96_NIDD_AT_Commands_Manual_V1.0 + return NSAPI_ERROR_PARAMETER; } - return err; + _at.lock(); + _at.cmd_start("AT+QCFGEXT=\"nipds\",1,"); + _at.write_hex_string((const char *)data, size); + _at.write_int(2 * size); + _at.cmd_stop_read_resp(); + nsapi_error_t err = _at.unlock_return_error(); + + return (err == NSAPI_ERROR_OK) ? size : err; } nsapi_size_or_error_t QUECTEL_BG96_ControlPlane_netif::recv(void *buffer, nsapi_size_t size) { _at.lock(); - _at.cmd_start_stop("QCFGEXT", "=", "%s%d", "nipdr", 0); - _at.resp_start("+QCFGEXT: "); + _at.cmd_start_stop("+QCFGEXT", "=", "%s%d", "nipdr", 0); + _at.resp_start("+QCFGEXT:"); // skip 3 params: "nipdr",, _at.skip_param(3); // get to @@ -32,20 +58,22 @@ nsapi_size_or_error_t QUECTEL_BG96_ControlPlane_netif::recv(void *buffer, nsapi_ _at.unlock(); return NSAPI_ERROR_WOULD_BLOCK; } + if ((nsapi_size_t)unread_length > size) { + tr_warn("recv %d/%d", size, unread_length); + unread_length = size; + } - _at.cmd_start_stop("QCFGEXT", "=", "%s%d", "nipdr", unread_length); - + _at.cmd_start_stop("+QCFGEXT", "=", "%s%d%d", "nipdr", unread_length, 1); _at.resp_start("+QCFGEXT:"); - // skip "nipdr" - _at.skip_param(); - int read_length = _at.read_int(); - _at.read_string((char *)buffer, read_length); + _at.skip_param(); // skip "nipdr" + nsapi_size_t read_length = _at.read_int(); + ssize_t read_len = _at.read_hex_string((char *)buffer, read_length); _at.resp_stop(); - nsapi_error_t err = _at.get_last_error(); - _at.unlock(); - if (err == NSAPI_ERROR_OK && read_length) { - return read_length; + nsapi_error_t err = _at.unlock_return_error(); + + if (err == NSAPI_ERROR_OK && read_len) { + return read_len; } return NSAPI_ERROR_WOULD_BLOCK;