Tibor Kiss

Important changes to forums and questions

All forums and questions are now archived. To start a new conversation or read the latest updates go to forums.mbed.com.

5 years, 9 months ago.

ATCmdParser usage in my own class

Hi guys,

first of all sorry but my CPP knowledge is not so much, I would like to create my own class for the HM-10 BLE module. For this I would like to use as a base the following library: https://os.mbed.com/teams/Seeed/code/BluetoothSerial/

I would like to use the ATCmdParser to send AT commands and receive the answers from the module, so I have modified the cpp file like:

HM_10.cpp

#include "HM_10.h"
#include <string.h>

 
#define LOG(args...)   // std::printf(args)
 
HM_10::HM_10(PinName tx, PinName rx) : _serial(tx, rx), _ATCmd(&_serial, "\r\n")
{   
    //_ATCmd = new ATCmdParser(&_serial, "\r\n");
    //_ATCmd.set_delimiter( "\r\n" );
}
 
void HM_10::setup()
{
    _serial.baud(HM_10_BAUD);
    _ATCmd.send("AT");
    if(_ATCmd.recv("OK")) {
        LOG("Serial connection to HM-10 is OK\r\n");
    } else { 
        LOG("Serial connection to HM-10 failed!\r\n");
        //return -1;
    }
}

HM_10.h

#ifndef __HM_10_H__
#define __HM_10_H__
 
#include "mbed.h"
#include <ATCmdParser.h>
 
#define HM_10_BAUD               9600
#define HM_10_TIMEOUT            10000
#define HM_10_EOL                "\r\n"
 
 
class HM_10 : public Stream {
public:
    HM_10(PinName tx, PinName rx);
    
    /**
     * Setup bluetooth module(serial port baud rate)
     */
    void setup();

protected:
    Serial          _serial;
    ATCmdParser    _ATCmd;
    uint8_t         _buf[64];
};
 
#endif // __HM_10_H__

And then I would like to use it the main.cpp

main.cpp

#include "mbed.h"
#include "string"
#include "HM_10.h"

int main()
{
    Serial UART = Serial(PA_2, PA_3, BAUD_RATE);
    HM_10 hm_10 = HM_10(PA_9, PA_10);
    
    hm_10.setup;

If I would like to compile the program I got an error: Error: A pointer to a bound function may only be used to call the function in "main.cpp", Line: 10, Col: 12 I am not sure is it correct how I created the constructor for the HM_10.

Could you please take a look at it and give me some advice. Thanks!

1 Answer

Zoltan Hudak
5 years, 9 months ago.

Hello, Tibor

When you create objects and then try to initialize them as below

    Serial UART = Serial(PA_2, PA_3, BAUD_RATE);
    HM_10 hm_10 = HM_10(PA_9, PA_10);

the compiler will first try to call the default constructor (with not parameters) to create an object. Since there is no such constructor defined anywhere it will silently try to create a very simple one which won't initialize anything. Then it will try to create another object by calling the constructor with parameters and finally it will try to assign the second object to the first one using the copy constructor (no one was defined by you so it will create a very simple one on your behalf which most likely won't do what you wanted to do). That's why it is more safe and efficient to declare and initialize in one step, like:

    Serial  UART(PA_2, PA_3, BAUD_RATE);
    HM_10   hm_10(PA_9, PA_10);

The HM_10 class is derived from Stream but you do not utilize that inheritance. So I'd suggest either to derive it from Serial as below (and then you do not need a Serial member) or remove that iheritance and keep the Serial member (use composition).

HM_10.h

#ifndef __HM_10_H__
#define __HM_10_H__
 
#include "mbed.h"
#include "ATCmdParser.h"
 
#define HM_10_BAUD       9600
#define HM_10_TIMEOUT    10000
#define HM_10_EOL        "\r\n"


class HM_10 : public Serial {
public:
    HM_10(PinName tx, PinName rx, int bitrate = HM_10_BAUD) : Serial(tx, rx, bitrate), _ATCmd(this, "\r\n") {}
    
    /**
     * Setup bluetooth module(serial port baud rate)
     */
    void setup(int bitrate = HM_10_BAUD);

protected:
    ATCmdParser    _ATCmd;
    uint8_t        _buf[64];
};

#endif // __HM_10_H__

HM_10.cpp

#include "HM_10.h"
#include <string.h>

void HM_10::setup(int bitrate /*=HM_10_BAUD*/)
{
    baud(bitrate);
    _ATCmd.send("AT");
    if(_ATCmd.recv("OK")) {
        printf("Serial connection to HM-10 is OK\r\n");
    } else { 
        printf("Serial connection to HM-10 failed!\r\n");
    }
}

main.cpp

#include "mbed.h"
#include "HM_10.h"

HM_10 hm_10(PA_9, PA_10);

int main()
{   
    hm_10.setup();

    while(1) {}
}

Accepted Answer

Hi Zoltán,

thanks for the help I really appreciate it. I have changed the constructor definition and basically, the main problem was that I forgot to brackets after hm_10.setup(); I would like to implement your further suggestions as well, but first I would like to study it a little bit. To understand better your proposal, would it be still possible to use the serial functions inside the class? I mean e.g. serial.printf("bla bla"); My second question would be, why it is better to move this from cpp file to the header file?

HM_10(PinName tx, PinName rx, int bitrate = HM_10_BAUD) : Serial(tx, rx, bitrate), _ATCmd(this, "\r\n") {}

And one more thing, I have improved the library further a little bit, the constructor looks like now:

HM_10::HM_10(PinName tx, PinName rx, PinName state, PinName en) : _serial(tx, rx), _ATCmd(&_serial, "\r\n"), _state(state), _enable(en), _stateINT(state)
{   
    _enable = 0;
    _stateINT.fall(callback(this, &HM_10::statusChangeHandler));
    _stateINT.rise(callback(this, &HM_10::statusChangeHandler));
    //_serial.attach(this, &HM_10::RX_interrupt, Serial::RxIrq);
    _rx_in = 0;
}

My question is, is it the right way to define handlers if the state pin status changes, and/or attach an interrupt handler if data is received?

Thanks your help in advance!

posted by Tibor Kiss 05 Feb 2019
  • Since HM_10 is actually an extended Serial, HM_10's constructor, destructor or methods are free to call any (public or protected) Serial function. But be cautious with printf. There are actually defined two such functions. One inherited from the Serial class and another (global) one defined in the mbed library and associated with the default serial port (the one available as mbed virtual serial port on the connected PC). In HM_10 function definitions use symple printf(..) (or this->printf(...) to call the inherited function. To call the global one, defined outside the HM_10/Serial class, prepend the scope operator, like ::printf(..). This will instruct the compiler to call the global printf function. If you decide go with composition (by keeping a Serial member and removing inherintance) you'll be equally able to call Serial functions (like _serial.printf(...), etc). Choose whatever is easier and more comfortable for you.
  • When you have a function or constructor/destructor with a very short or empty body you can make it inline by putting it's definition into the header file (or by declaring it as inline and keeping it in the cpp file ). The compiler will then substitute the function body for the function call, thus eliminating the overhead. The inline code does occupy space, but if the function is small, this can actually take less space than the code generated to do an ordinary function call (pushing arguments on the stack and doing the CALL).
  • In my opinion, if you plan to connect status pins then it's logical to utilize them to handle the related intrrupts, actually as you did. I think you are on the right way. Good luck.
posted by Zoltan Hudak 05 Feb 2019

Thanks for your helpful feedback. I have continued to work on the HM_10.ccp and header file, the constructor how it is created almost working correctly, one think is not okay, and I have searched a lot but I cannot find the answer/solution, so the constructor looks like:

HM_10::HM_10(PinName tx, PinName rx, PinName state, PinName en) : _serial(tx, rx), _ATCmd(&_serial, "\r\n"), _state(state), _enable(en), _stateINT(state)
{   
    _enable = 0;
    _stateINT.fall(callback(this, &HM_10::statusChangeHandler));
    _stateINT.rise(callback(this, &HM_10::statusChangeHandler));
    _serial.attach(this, &HM_10::RX_interrupt, Serial::RxIrq);
    _rx_in = 0;
}

In that case I have warning: Warning: Function "mbed::SerialBase::attach(T *, void (T::*)(), mbed::SerialBase::IrqType) [with T=HM_10]" (declared at line 117 of "/extras/mbed/drivers/SerialBase.h") was declared "deprecated" in "HM_10.cpp", Line: 12, Col: 14

So I have changed it like the "rise" and "fall":

    _serial.attach(callback(this, &HM_10::RX_interrupt, Serial::RxIrq));

But in that case I get an error: Error: No instance of overloaded function "callback" matches the argument list in "HM_10.cpp", Line: 12, Col: 21

If I ignor the warning in case of the first version (without callback) and flash the device it is crashing after an Rx interrupt happens. What could be wrong? Just a note the "rise" and "fall" interrupts are working correctly, only the serial rx interrupt has problem.

Thanks your help in advance!

posted by Tibor Kiss 09 Feb 2019
  • If you dont like the warning try 

    _serial.attach(callback(this, &HM_10::RX_interrupt), Serial::RxIrq);
  • Mutex methods shall not be called from interrupt service routines. In the current version of Mbed OS, if you attempt to use a mutex from within an ISR, it will treat that as a fatal system error. Read here for more details. For example, if you try 

void HM_10::RX_interrupt()
{
    char c = _serial.getc();
    //..
}

then it will result in a fatal system error because Serial::getc(), which is inherited from Stream, locks the Mutex (by calling the lock() function).

posted by Zoltan Hudak 09 Feb 2019