Merge pull request #12122 from AnttiKauppila/visibility_fixes

ATHandler refactoring

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);

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;

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

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)
 {
 }

features/cellular/framework/API/ATHandler.h

@@ -77,7 +77,8 @@ public:
      *  @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 @@ public:
      */
     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 @@ public:
      */
     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<void()> 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 @@ public:
      */
     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 @@ public: // just for debugging
      */
     static void set_debug_list(bool debug_on);
 
-private:
+private: //Private structs & enums
-
-    // should fit any prefix and int
-    char _recv_buff[BUFF_SIZE];
-    // reading position
-    size_t _recv_len;
-    // reading length
-    size_t _recv_pos;
-
-    // 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;
     };
 
-    // tag to stop response scope
+    struct oob_t {
-    tag_t _resp_stop;
+        const char *prefix;
-    // tag to stop information response scope
+        int prefix_len;
-    tag_t _info_stop;
+        Callback<void()> cb;
-    // tag to stop element scope
+        oob_t *next;
-    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
+    // resp_type: the part of the response that doesn't include the information response (+CMD1,+CMD2..)
-    char _delimiter;
+    //            ends with OK or (CME)(CMS)ERROR
-    // set true on prefix match -> indicates start of an information response or of an element
+    // info_type: the information response part of the response: starts with +CMD1 and ends with CRLF
-    bool _prefix_matched;
+    //            information response contains parameters or subsets of parameters (elements), both separated by comma
-    // set true on urc match
+    // elem_type: subsets of parameters that are part of information response, its parameters are separated by comma
-    bool _urc_matched;
+    enum ScopeType {
-    // set true on (CME)(CMS)ERROR
+        RespType,
-    bool _error_found;
+        InfoType,
-    // Max length of OK,(CME)(CMS)ERROR and URCs
+        ElemType,
-    size_t _max_resp_length;
+        NotSet
+    };
 
-    // prefix set during resp_start and used to try matching possible information responses
+private: //Private functions
-    char _info_resp_prefix[BUFF_SIZE];
+    void event();
-    bool _debug_on;
-    bool _cmd_start;
-    bool _use_delimiter;
 
-    // time when a command or an URC processing was started
+    /** Increase reference count. Used for counting references to this instance.
-    uint64_t _start_time;
+     *  Note that this should be used with care, if the ATHandler was taken into use
-    // eventqueue event id
+     *  with get_instance()
-    int _event_id;
+     */
+    void inc_ref_count();
 
-    char _cmd_buffer[BUFF_SIZE];
+    /** 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();
+
+    /** 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 @@ private:
     //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 @@ private:
     // 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 @@ private:
         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

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 <ATHandler> lock(*this);