From 36ca4d6516d9557327453aa2dc2da32786a31078 Mon Sep 17 00:00:00 2001 From: Antti Kauppila Date: Mon, 16 Dec 2019 14:18:21 +0200 Subject: [PATCH 1/2] ATHandler relocated ATHandler is part of our API so header file was moved under API folder and .cpp file was moved under device/ folder ATHandler is used in both AT and PPP mode so it has been in slightly wrong place at the beginning. --- .../framework/{AT => device}/athandler/athandlertest.cpp | 0 .../framework/{AT => device}/athandler/unittest.cmake | 4 ++-- features/cellular/framework/{AT => API}/ATHandler.h | 0 features/cellular/framework/{AT => device}/ATHandler.cpp | 0 4 files changed, 2 insertions(+), 2 deletions(-) rename UNITTESTS/features/cellular/framework/{AT => device}/athandler/athandlertest.cpp (100%) rename UNITTESTS/features/cellular/framework/{AT => device}/athandler/unittest.cmake (89%) rename features/cellular/framework/{AT => API}/ATHandler.h (100%) rename features/cellular/framework/{AT => device}/ATHandler.cpp (100%) diff --git a/UNITTESTS/features/cellular/framework/AT/athandler/athandlertest.cpp b/UNITTESTS/features/cellular/framework/device/athandler/athandlertest.cpp similarity index 100% rename from UNITTESTS/features/cellular/framework/AT/athandler/athandlertest.cpp rename to UNITTESTS/features/cellular/framework/device/athandler/athandlertest.cpp diff --git a/UNITTESTS/features/cellular/framework/AT/athandler/unittest.cmake b/UNITTESTS/features/cellular/framework/device/athandler/unittest.cmake similarity index 89% rename from UNITTESTS/features/cellular/framework/AT/athandler/unittest.cmake rename to UNITTESTS/features/cellular/framework/device/athandler/unittest.cmake index 164ea716ff6..e2ba6442544 100644 --- a/UNITTESTS/features/cellular/framework/AT/athandler/unittest.cmake +++ b/UNITTESTS/features/cellular/framework/device/athandler/unittest.cmake @@ -15,12 +15,12 @@ set(unittest-includes ${unittest-includes} # Source files set(unittest-sources - ../features/cellular/framework/AT/ATHandler.cpp + ../features/cellular/framework/device/ATHandler.cpp ) # Test files set(unittest-test-sources - features/cellular/framework/AT/athandler/athandlertest.cpp + features/cellular/framework/device/athandler/athandlertest.cpp stubs/EventQueue_stub.cpp stubs/FileHandle_stub.cpp stubs/us_ticker_stub.cpp diff --git a/features/cellular/framework/AT/ATHandler.h b/features/cellular/framework/API/ATHandler.h similarity index 100% rename from features/cellular/framework/AT/ATHandler.h rename to features/cellular/framework/API/ATHandler.h diff --git a/features/cellular/framework/AT/ATHandler.cpp b/features/cellular/framework/device/ATHandler.cpp similarity index 100% rename from features/cellular/framework/AT/ATHandler.cpp rename to features/cellular/framework/device/ATHandler.cpp From 5553d0acd3c46d946b1e8be96f117c91fd48b7fd Mon Sep 17 00:00:00 2001 From: Antti Kauppila Date: Mon, 16 Dec 2019 15:45:52 +0200 Subject: [PATCH 2/2] ATHandler class refactor Refactored ATHandler class to have clear private and public elements Also removed virtuality from ATHandler Unittests updated to reflect changes --- .../at_cellulardevicetest.cpp | 15 +- .../device/athandler/athandlertest.cpp | 41 ---- UNITTESTS/stubs/ATHandler_stub.cpp | 10 +- features/cellular/framework/API/ATHandler.h | 226 +++++++++--------- .../cellular/framework/device/ATHandler.cpp | 23 +- 5 files changed, 133 insertions(+), 182 deletions(-) diff --git a/UNITTESTS/features/cellular/framework/AT/at_cellulardevice/at_cellulardevicetest.cpp b/UNITTESTS/features/cellular/framework/AT/at_cellulardevice/at_cellulardevicetest.cpp index 610517f5ac6..d0a1630dc2d 100644 --- a/UNITTESTS/features/cellular/framework/AT/at_cellulardevice/at_cellulardevicetest.cpp +++ b/UNITTESTS/features/cellular/framework/AT/at_cellulardevice/at_cellulardevicetest.cpp @@ -78,12 +78,10 @@ TEST_F(TestAT_CellularDevice, test_AT_CellularDevice_get_at_handler) AT_CellularDevice *dev2 = new AT_CellularDevice(&fh1); // AT fh1 ref count 3 EXPECT_TRUE(dev2->open_information(&fh1)); // AT fh1 ref count 4 ATHandler *at = dev2->get_at_handler(); // AT fh1 ref count 5 - EXPECT_EQ(at->get_ref_count(), 6); delete dev2; // AT fh1 2 refs deleted -> ref count 3 - EXPECT_EQ(at->get_ref_count(), 4); + AT_CellularDevice dev3(&fh1); // AT fh1 ref count 4 EXPECT_TRUE(dev3.release_at_handler(at) == NSAPI_ERROR_OK); // AT fh1 ref count 3 - EXPECT_EQ(ATHandler_stub::ref_count, 4); } TEST_F(TestAT_CellularDevice, test_AT_CellularDevice_open_network) @@ -272,7 +270,6 @@ TEST_F(TestAT_CellularDevice, test_AT_CellularDevice_create_delete_context) AT_CellularDevice *dev = new AT_CellularDevice(&fh1); ATHandler *at = dev->get_at_handler(); - EXPECT_EQ(at->get_ref_count(), 1); EXPECT_TRUE(dev->release_at_handler(at) == NSAPI_ERROR_OK); CellularContext *ctx = dev->create_context(NULL); @@ -280,12 +277,9 @@ TEST_F(TestAT_CellularDevice, test_AT_CellularDevice_create_delete_context) dev = new AT_CellularDevice(&fh1); at = dev->get_at_handler(); - EXPECT_EQ(at->get_ref_count(), 1); ctx = dev->create_context(NULL); CellularContext *ctx1 = dev->create_context(&fh1); - EXPECT_EQ(at->get_ref_count(), 3); CellularContext *ctx2 = dev->create_context(&fh1); - EXPECT_EQ(at->get_ref_count(), 4); EXPECT_TRUE(ctx); EXPECT_TRUE(ctx1); @@ -296,20 +290,13 @@ TEST_F(TestAT_CellularDevice, test_AT_CellularDevice_create_delete_context) EXPECT_TRUE(xx); dev->delete_context(ctx); - EXPECT_EQ(at->get_ref_count(), 3); dev->delete_context(ctx1); - EXPECT_EQ(at->get_ref_count(), 2); dev->delete_context(NULL); - EXPECT_EQ(at->get_ref_count(), 2); dev->delete_context(ctx2); - EXPECT_EQ(at->get_ref_count(), 1); ctx = dev->create_context(NULL); - EXPECT_EQ(at->get_ref_count(), 2); ctx1 = dev->create_context(&fh1); - EXPECT_EQ(at->get_ref_count(), 3); ctx2 = dev->create_context(&fh1); - EXPECT_EQ(at->get_ref_count(), 4); EXPECT_TRUE(dev->release_at_handler(at) == NSAPI_ERROR_OK); EXPECT_TRUE(ctx); EXPECT_TRUE(ctx1); diff --git a/UNITTESTS/features/cellular/framework/device/athandler/athandlertest.cpp b/UNITTESTS/features/cellular/framework/device/athandler/athandlertest.cpp index 40cd643941c..d292935d3cf 100644 --- a/UNITTESTS/features/cellular/framework/device/athandler/athandlertest.cpp +++ b/UNITTESTS/features/cellular/framework/device/athandler/athandlertest.cpp @@ -105,7 +105,6 @@ TEST_F(TestATHandler, test_ATHandler_list) ATHandler::set_debug_list(false); ATHandler *at1 = ATHandler::get_instance(&fh1, que, 0, ",", 0, 0); - EXPECT_TRUE(at1->get_ref_count() == 1); ATHandler::set_at_timeout_list(1000, false); ATHandler::set_debug_list(true); @@ -113,14 +112,11 @@ TEST_F(TestATHandler, test_ATHandler_list) EXPECT_TRUE(ATHandler::get_instance(NULL, que, 0, ",", 0, 0) == NULL); ATHandler *at2 = ATHandler::get_instance(&fh1, que, 0, ",", 0, 0); - EXPECT_TRUE(at1->get_ref_count() == 2); - EXPECT_TRUE(at2->get_ref_count() == 2); ATHandler::set_at_timeout_list(2000, true); ATHandler::set_debug_list(false); EXPECT_TRUE(at1->close() == NSAPI_ERROR_OK); - EXPECT_TRUE(at2->get_ref_count() == 1); EXPECT_TRUE(at2->close() == NSAPI_ERROR_OK); ATHandler::set_at_timeout_list(1000, false); @@ -204,43 +200,6 @@ TEST_F(TestATHandler, test_ATHandler_get_last_device_error) EXPECT_TRUE(0 == at.get_last_device_error().errCode); } -TEST_F(TestATHandler, test_ATHandler_inc_ref_count) -{ - EventQueue que; - FileHandle_stub fh1; - - ATHandler at(&fh1, que, 0, ","); - at.inc_ref_count(); -} - -TEST_F(TestATHandler, test_ATHandler_dec_ref_count) -{ - EventQueue que; - FileHandle_stub fh1; - - ATHandler at(&fh1, que, 0, ","); - at.dec_ref_count(); -} - -TEST_F(TestATHandler, test_ATHandler_get_ref_count) -{ - EventQueue que; - FileHandle_stub fh1; - - ATHandler at(&fh1, que, 0, ","); - EXPECT_TRUE(1 == at.get_ref_count()); - - at.inc_ref_count(); - EXPECT_TRUE(2 == at.get_ref_count()); - - at.inc_ref_count(); - EXPECT_TRUE(3 == at.get_ref_count()); - - at.dec_ref_count(); - at.dec_ref_count(); - EXPECT_TRUE(1 == at.get_ref_count()); -} - TEST_F(TestATHandler, test_ATHandler_set_at_timeout) { EventQueue que; diff --git a/UNITTESTS/stubs/ATHandler_stub.cpp b/UNITTESTS/stubs/ATHandler_stub.cpp index d48e02102ab..6f7997e8af3 100644 --- a/UNITTESTS/stubs/ATHandler_stub.cpp +++ b/UNITTESTS/stubs/ATHandler_stub.cpp @@ -111,6 +111,11 @@ bool ATHandler::get_debug() const return ATHandler_stub::debug_on; } +void ATHandler::set_debug_list(bool debug_on) +{ + ATHandler_stub::debug_on = debug_on; +} + ATHandler::~ATHandler() { ATHandler_stub::urc_handlers.clear(); @@ -409,11 +414,6 @@ void ATHandler::set_at_timeout_list(uint32_t timeout_milliseconds, bool default_ ATHandler_stub::default_timeout = default_timeout; } -void ATHandler::set_debug_list(bool debug_on) -{ - ATHandler_stub::debug_on = debug_on; -} - void ATHandler::set_send_delay(uint16_t send_delay) { } diff --git a/features/cellular/framework/API/ATHandler.h b/features/cellular/framework/API/ATHandler.h index b41aae6e539..1d54f226f14 100644 --- a/features/cellular/framework/API/ATHandler.h +++ b/features/cellular/framework/API/ATHandler.h @@ -77,7 +77,8 @@ class ATHandler { * @param send_delay the minimum delay in ms between the end of last response and the beginning of a new command */ ATHandler(FileHandle *fh, events::EventQueue &queue, uint32_t timeout, const char *output_delimiter, uint16_t send_delay = 0); - virtual ~ATHandler(); + + ~ATHandler(); /** Return used file handle. * @@ -146,24 +147,6 @@ class ATHandler { */ device_err_t get_last_device_error() const; - /** Increase reference count. Used for counting references to this instance. - * Note that this should be used with care, if the ATHandler was taken into use - * with get_instance() - */ - void inc_ref_count(); - - /** Decrease reference count. Used for counting references to this instance. - * Note that this should be used with care, if the ATHandler was taken into use - * with get_instance() - */ - void dec_ref_count(); - - /** Get the current reference count. Used for counting references to this instance. - * - * @return current reference count - */ - int get_ref_count(); - /** Set timeout in milliseconds for AT commands * * @param timeout_milliseconds Timeout in milliseconds @@ -227,56 +210,12 @@ class ATHandler { */ void set_baud(int baud_rate); -protected: - void event(); -#if defined AT_HANDLER_MUTEX && defined MBED_CONF_RTOS_PRESENT - rtos::Mutex _fileHandleMutex; - rtos::ConditionVariable _oobCv; -#endif - FileHandle *_fileHandle; -private: - /** Remove urc handler from linked list of urc's - * - * @param prefix Register urc prefix for callback. Urc could be for example "+CMTI: " - */ - void remove_urc_handler(const char *prefix); - - void set_error(nsapi_error_t err); - - events::EventQueue &_queue; - nsapi_error_t _last_err; - int _last_3gpp_error; - device_err_t _last_at_err; - uint16_t _oob_string_max_length; - char *_output_delimiter; - - struct oob_t { - const char *prefix; - int prefix_len; - Callback cb; - oob_t *next; - }; - oob_t *_oobs; - uint32_t _at_timeout; - uint32_t _previous_at_timeout; - - uint16_t _at_send_delay; - uint64_t _last_response_stop; - - int32_t _ref_count; - bool _is_fh_usable; - - static ATHandler *_atHandlers; - - //************************************* -public: - /** Starts the command writing by clearing the last error and writing the given command. * In case of failure when writing, the last error is set to NSAPI_ERROR_DEVICE_ERROR. * * @param cmd AT command to be written to modem */ - virtual void cmd_start(const char *cmd); + void cmd_start(const char *cmd); /** * @brief cmd_start_stop Starts an AT command, writes given variadic arguments and stops the command. Use this @@ -323,8 +262,6 @@ class ATHandler { */ nsapi_error_t at_cmd_discard(const char *cmd, const char *cmd_chr, const char *format = "", ...); -public: - /** Writes integer type AT command subparameter. Starts with the delimiter if not the first param after cmd_start. * In case of failure when writing, the last error is set to NSAPI_ERROR_DEVICE_ERROR. * @@ -517,64 +454,61 @@ class ATHandler { */ static void set_debug_list(bool debug_on); -private: +private: //Private structs & enums + struct tag_t { + char tag[7]; + size_t len; + bool found; + }; - // should fit any prefix and int - char _recv_buff[BUFF_SIZE]; - // reading position - size_t _recv_len; - // reading length - size_t _recv_pos; + struct oob_t { + const char *prefix; + int prefix_len; + Callback cb; + oob_t *next; + }; // resp_type: the part of the response that doesn't include the information response (+CMD1,+CMD2..) // ends with OK or (CME)(CMS)ERROR // info_type: the information response part of the response: starts with +CMD1 and ends with CRLF // information response contains parameters or subsets of parameters (elements), both separated by comma // elem_type: subsets of parameters that are part of information response, its parameters are separated by comma - enum ScopeType {RespType, InfoType, ElemType, NotSet}; - void set_scope(ScopeType scope_type); - ScopeType _current_scope; - - struct tag_t { - char tag[7]; - size_t len; - bool found; + enum ScopeType { + RespType, + InfoType, + ElemType, + NotSet }; - // tag to stop response scope - tag_t _resp_stop; - // tag to stop information response scope - tag_t _info_stop; - // tag to stop element scope - tag_t _elem_stop; - // reference to the stop tag of current scope (resp/info/elem) - tag_t *_stop_tag; +private: //Private functions + void event(); - // delimiter between parameters and also used for delimiting elements of information response - char _delimiter; - // set true on prefix match -> indicates start of an information response or of an element - bool _prefix_matched; - // set true on urc match - bool _urc_matched; - // set true on (CME)(CMS)ERROR - bool _error_found; - // Max length of OK,(CME)(CMS)ERROR and URCs - size_t _max_resp_length; + /** Increase reference count. Used for counting references to this instance. + * Note that this should be used with care, if the ATHandler was taken into use + * with get_instance() + */ + void inc_ref_count(); - // prefix set during resp_start and used to try matching possible information responses - char _info_resp_prefix[BUFF_SIZE]; - bool _debug_on; - bool _cmd_start; - bool _use_delimiter; + /** Decrease reference count. Used for counting references to this instance. + * Note that this should be used with care, if the ATHandler was taken into use + * with get_instance() + */ + void dec_ref_count(); - // time when a command or an URC processing was started - uint64_t _start_time; - // eventqueue event id - int _event_id; + /** Get the current reference count. Used for counting references to this instance. + * + * @return current reference count + */ + int get_ref_count(); - char _cmd_buffer[BUFF_SIZE]; + /** Remove urc handler from linked list of urc's + * + * @param prefix Register urc prefix for callback. Urc could be for example "+CMTI: " + */ + void remove_urc_handler(const char *prefix); + + void set_error(nsapi_error_t err); -private: //Handles the arguments from given variadic list void handle_args(const char *format, std::va_list list); @@ -584,7 +518,6 @@ class ATHandler { //Checks that ATHandler does not have a pending error condition and filehandle is usable bool ok_to_proceed(); -private: // Gets char from receiving buffer. // Resets and fills the buffer if all are already read (receiving position equals receiving length). // Returns a next char or -1 on failure (also sets error flag) @@ -619,6 +552,7 @@ class ATHandler { // Checks if receiving buffer contains OK, ERROR, URC or given prefix. void resp(const char *prefix, bool check_urc); + void set_scope(ScopeType scope_type); ScopeType get_scope(); @@ -659,7 +593,77 @@ class ATHandler { AT_RX, AT_TX }; + void debug_print(const char *p, int len, ATType type); + +private: //Member variables + +#if defined AT_HANDLER_MUTEX && defined MBED_CONF_RTOS_PRESENT + rtos::Mutex _fileHandleMutex; + rtos::ConditionVariable _oobCv; +#endif + FileHandle *_fileHandle; + + events::EventQueue &_queue; + nsapi_error_t _last_err; + int _last_3gpp_error; + device_err_t _last_at_err; + uint16_t _oob_string_max_length; + char *_output_delimiter; + + oob_t *_oobs; + uint32_t _at_timeout; + uint32_t _previous_at_timeout; + + uint16_t _at_send_delay; + uint64_t _last_response_stop; + + int32_t _ref_count; + bool _is_fh_usable; + + static ATHandler *_atHandlers; + + // should fit any prefix and int + char _recv_buff[BUFF_SIZE]; + // reading position + size_t _recv_len; + // reading length + size_t _recv_pos; + + ScopeType _current_scope; + + // tag to stop response scope + tag_t _resp_stop; + // tag to stop information response scope + tag_t _info_stop; + // tag to stop element scope + tag_t _elem_stop; + // reference to the stop tag of current scope (resp/info/elem) + tag_t *_stop_tag; + + // delimiter between parameters and also used for delimiting elements of information response + char _delimiter; + // set true on prefix match -> indicates start of an information response or of an element + bool _prefix_matched; + // set true on urc match + bool _urc_matched; + // set true on (CME)(CMS)ERROR + bool _error_found; + // Max length of OK,(CME)(CMS)ERROR and URCs + size_t _max_resp_length; + + // prefix set during resp_start and used to try matching possible information responses + char _info_resp_prefix[BUFF_SIZE]; + bool _debug_on; + bool _cmd_start; + bool _use_delimiter; + + // time when a command or an URC processing was started + uint64_t _start_time; + // eventqueue event id + int _event_id; + + char _cmd_buffer[BUFF_SIZE]; }; } // namespace mbed diff --git a/features/cellular/framework/device/ATHandler.cpp b/features/cellular/framework/device/ATHandler.cpp index ed60d2c699e..c24eb9152ab 100644 --- a/features/cellular/framework/device/ATHandler.cpp +++ b/features/cellular/framework/device/ATHandler.cpp @@ -137,17 +137,6 @@ void ATHandler::set_at_timeout_list(uint32_t timeout_milliseconds, bool default_ singleton_unlock(); } -void ATHandler::set_debug_list(bool debug_on) -{ - ATHandler *atHandler = _atHandlers; - singleton_lock(); - while (atHandler) { - atHandler->set_debug(debug_on); - atHandler = atHandler->_nextATHandler; - } - singleton_unlock(); -} - bool ATHandler::ok_to_proceed() { if (_last_err != NSAPI_ERROR_OK) { @@ -211,6 +200,7 @@ ATHandler::ATHandler(FileHandle *fh, EventQueue &queue, uint32_t timeout, const set_file_handle(fh); } + void ATHandler::set_debug(bool debug_on) { _debug_on = debug_on; @@ -221,6 +211,17 @@ bool ATHandler::get_debug() const return _debug_on; } +void ATHandler::set_debug_list(bool debug_on) +{ + ATHandler *atHandler = _atHandlers; + singleton_lock(); + while (atHandler) { + atHandler->set_debug(debug_on); + atHandler = atHandler->_nextATHandler; + } + singleton_unlock(); +} + ATHandler::~ATHandler() { ScopedLock lock(*this);