I'm trying to encapsulate an AnalogIn and Serial object in a class for an ultrasonic rangefinder that has several output options.  The idea is to have the user select the mode and pins with the parameters and then the query function will return the output from the selected output option.  For example, the user might create SonarRangeFinder SRF(0, p13, p14) and then call Query() and the method will return the value from the analog input of the range finder.

There are several errors in here, but the one I can't get around is "Nonstandard form for taking the address of a member function" and "expression must have class type" at "x = AInput.read();".  Does anyone know how to correct these errors?

 

#ifndef SonarRangeFinderLibrary
#define SonarRangeFinderLibrary

#include "mbed.h"

class SonarRangeFinder
{
private:
DigitalOut SOutput(PinName);
AnalogIn AInput(PinName);
Serial SInput(PinName);
int Mode;
int Unit;

public:
SonarRangeFinder(int, PinName, PinName);
void SelectUnit(int);
float Query();
};

#endif

 

#include "SonarRangeFinderB.h"

SonarRangeFinder::SonarRangeFinder(int choice, PinName tx, PinName rx)
{
DigitalOut SOutput(tx);
AnalogIn AInput(rx);
Serial SInput(tx, rx);
Mode = choice;
Unit = 0;
}

void SonarRangeFinder::SelectUnit(int choice)
{
Unit = choice;
}

float SonarRangeFinder::Query()
{
char response[5];
float x;
switch (Mode)
{
case 0:
x = AInput.read();
break;
case 1:
SOutput.write(1);
wait_ms(1);//hold pin high for at least 20 us to signal a serial query
SOutput.write(0);
wait_ms(100);//wait for a response
response = SInput.scanf();
x = atof(response);
break;
}
return x;
}

Hi Jonathan,

The main problem here is the way you are creating the objects DigitalOut, AnalogIn and Serial. These want to be part of the Sonar object, so you actually have to pass the constructor variables (e.g. the pin names) to them as part of the Sonar constructor. By example:

class SonarRangeFinder
{
private:
DigitalOut SOutput(PinName);
AnalogIn AInput(PinName);
Serial SInput(PinName);

becomes

class SonarRangeFinder
{
private:
DigitalOut SOutput;  // no constructor arguments here
AnalogIn AInput;     
Serial SInput;

and the body of your sonar constructor:

SonarRangeFinder::SonarRangeFinder(int choice, PinName tx, PinName rx)
{
DigitalOut SOutput(tx);
AnalogIn AInput(rx);
Serial SInput(tx, rx);

becomes

SonarRangeFinder::SonarRangeFinder(int choice, PinName tx, PinName rx) : SOutput(tx), AInput(rx), SInput(tx, rx)
{

Note the notation which says how to construct the objects Sonar contains, before calling the Sonar constructor itself.

This will then initialise them in the way you are intending.

A couple of other things from this code:

response = SInput.scanf();
x = atof(response);

isn't a valid way to use scanf (or at least it won't give you the results you want!). You probably should try something like:

SInput.scanf("%f", &x);

Also, you have multiple objects trying to use the same pins. That won't really work, as the pins have to be configured to do something (e.g. be a serial port, or an analog port). Not sure if this is a requirement of what you are trying to do, but it'd be worth looking at a bit more.

Simon

removed post.

Jonathan Grubb wrote:
The documentation says NOT_CONNECTED is a valid argument for Serial, but I get the error NOT_CONNECTED is undefined; how do I use it?

Hi Jonathan

I think the documentation is out of date (having been written during the beta phase of the mbed's development). You can use the symbol NC with a serial device rather than NOT_CONNECTED eg.

Serial rxOnly(NC, p14);

I'm not sure you can apply this to the analog - give it a go. If not, you might need to dynamically create the analog object if you don't always want it.

Regards
Daniel

 

Thanks everyone, it looks like the code will work (it compiles at least).  I'm waiting for testing to get back to me.

I can also verify that AnalogIn obj(NC) will compile, all that's left to do is see if it actually does what it looks like it does.

Thanks.

Hi Jonathan,

Good to see you are making progress. Just seen this last post:

AnalogIn obj(NC); is not a valid thing to do.

The NC is to allow you to create an interface on which not all pins are used, for example not using the Serial rx channel, or the SPI miso channel. For AnalogIn it is not applicable as it is only one input, and therefore doesn't make sense not to have it.

I also noticed you changed e.g

AInput.read();

to

AnalogIn::read();

This is not correct, and will not do what you are intending. You need to call the method (read) on an instance (AInput) of the class (AnalogIn), not the class itself. I suspect you have changed this to try and stop some compiler warnings, but unfortunately it is not going to give you the right result!

As a simple example of a full class to demonstrate how this all works:

Flasher.h

#ifndef MBED_FLASHER_H
#define MBED_FLASHER_H

#include "mbed.h"

class Flasher {
public:
    Flasher(PinName pin);
    void flash(int n);
  
private:  
    DigitalOut _pin;
};

#endif

Flasher.cpp

#include "Flasher.h"
#include "mbed.h"

Flasher::Flasher(PinName pin) : _pin(pin) {
    _pin.write(0);
}

void Flasher::flash(int n) {
    for(int i=0; i<n*2; n++) {
        _pin.write(!_pin.read());
        wait(0.2);
    }
}

Hopefully this example will show a good template of what you need to do (it is based on an example I wrote to test the new cookbook, see /cookbook/Writing-a-library with betamode enabled)

btw, it is also possible to "publish" your program, which makes it much easier for others to import, modify, and post back. You then just need to post a link in the forum with perhaps the bits of code you are interested in, not the whole code.

Good luck!

Simon

Daniel Peter wrote:
I'm not sure you can apply this to the analog - give it a go. If not, you might need to dynamically create the analog object if you don't always want it.

Hi Jonathan

I'm hoping Simon will correct me if I send you up the wrong tree! This code snippet optionally creates the AnalogIn ... don't try reading from it if not created though.

    AnalogIn *input;
    PinName analogPin = p12;
    if (analogPin != NC) {
        input = new AnalogIn(analogPin);
        float value = input->read();
    }

You could pass an analog pin name in your constructor, and if NC is passed in then you don't create the AnalogIn object with the code above.

Regards
Daniel

Hmmm, I was basing the code off of some other classes that I found online; I was trying to make a child class of the two, but could not find an example of this and just went on what made sense.  You're right about the compiler error, it was saying the read method was ambiguous.  Is the solution you gave me a "child class"?  If not could you tell me the difference?

Also, what happens when AnalogIn obj(NC)?  The program compiles, and it isn't given a pin; it doesn't seem like it would do anything at all.  Is there some sort of runtime error that would occur?

Thanks again, you've been immensely helpful.

hello, I'm working on the same project.

Our latest code is at ClassFile

I'm concerned about the Serial.readable() method calls.  In some other programs it did not work as expected.  We structured it to execute a read operation if readable returned true.  However the readable method always returned false no matter what measures we took to put data in the buffer before hand.  Is there more explanation available that the one in the docs (I can't seem to find anyone who has had the same problem either)

Thanks

Do you have an example piece of code we could have a look at? The Serial class is in the queue to be looked at quite shortly.

Certainly.  To save space, I omitted some of the more boring code.  This code is for an IMU protocol, the IMU accepts a 1 byte command and returns 32 bytes of response data.  We call readable() in the last method to determine if the IMU has responded.  Since it didn't work, we omitted that check and just allowed the program to block until read was able to finish.  This gave us results, but concern remains over future usage of readable().

 

#include "mbed.h"
#include "stdint.h"

Serial IMU(p9, p10);
Serial Computer(p13, p14);
Timer LoopCon;

char Data[31];

struct DataStr
{/*hold reply field here*/} IMUData;

void QueryIMU(char Command, int PacketSize);
bool CheckSum(char* DataPtr, char* CheckPtr);
void RecordTime(char* TimePtr);
void GetFloat(char* BytePtr, float* FloatPtr);
void GetULong(char* BytePtr, unsigned long* ULongPtr);

int main()
{
    IMU.baud(115200);
    Computer.baud(38400);
    wait_ms(10);
    
    while (1) {
        
        QueryIMU(0xC2, 31);
        
        if (Computer.writeable())
        {/*printf the struct to the computer*/}

        LoopCon.start();
        while (LoopCon.read_ms() < 1000);
        LoopCon.stop();
        LoopCon.reset();
        //instead of using the timer, could we just call "wait(1);"?
    }
}

void QueryIMU(char Command, int PacketSize)
{
    IMU.putc(Command);
    if (IMU.readable())                                  //readable called here
    {
        for (int i = 0; i < PacketSize; i++)
        {
            Data[i] = IMU.getc();
        }
    }
    if (Data[0] == Command)
    {
        if (CheckSum(&Data[0], &Data[PacketSize - 2]))
        {
            IMUData.Command = Data[0];
            GetFloat(&Data[1], &IMUData.XAccel);
            GetFloat(&Data[5], &IMUData.YAccel);
            GetFloat(&Data[9], &IMUData.ZAccel);
            GetFloat(&Data[13], &IMUData.XAngRate);
            GetFloat(&Data[17], &IMUData.YAngRate);
            GetFloat(&Data[21], &IMUData.ZAngRate);
            RecordTime(&Data[25]);
        }
    }
}

Thanks for the help

It might be just a typo in this copy of your code, but you are actually calling the .readable() method of your other Serial interface (Computer.readable()) instead of IMU.readable() in your QueryIMU method.

Regards
Neni

Oh, you're right, that is a typo.  Fixed it.

Does it work correctly now?

[And many thanks to Nenad for replying]

No, it does not work.  Is it supposed to return whether or not there are bytes in the buffer?  The call to readable always returns false, and we have verified that the IMU is actually writing data to the port.  Is it checking a lower buffer that is emtied immediately?

Even changing the code to:

 

void QueryIMU(char Command, int PacketSize)
{
    if (IMU.writeable())
    {
        IMU.putc(Command);
    }
    for (int i = 0; i < PacketSize; i++)
    {
        Data[i] = IMU.getc();
        if (IMU.readable())
        {
            test = 1;
        }
    }
    if (Data[0] == Command)
    {
    ...

(the size of the packet is 32) the test variable always returns false.  I have tried tests like this on other (unrelated) code too and readable always returns false.

 

 

Thanks for the help everyone, it is very appreciated.

Hi Jonathan,

Looking at this:

for (int i = 0; i < PacketSize; i++) {
    Data[i] = IMU.getc();
    if (IMU.readable()) {
        test = 1;
    }
}

I'd suggest that readable() would never be true. The logic is basically:

Wait for a character to be available and then get it (getc), then check once if there are any more characters. Repeat for Packet size.

So assuming no characters already in the serial buffer, getc() will wait until the first character arrives, and when it does, put it in to Data[0]. Then you test readable, which will be false as there are no characters in the buffer (in the time between the getc() and that test, another one wont have arrived). And then you loop round and wait on getc() again, and the process continues.

I therefore think it is actually giving the right behaviour, just not what you intended?

In your previous code, you called readable just after your putc(). Again, this would always fail as no reply would have been recieved from the IMU in that short time.

It would be good to know therefore if:

• It is just a simple programming mistake/oversight
• The semantics of readable() and Serial are not clear enough, so you thought they did something other than what they do; so we should improve this explanation (if you could explain how you understood them, that would be interesting)
• I've overlooked or misunderstood something about your setup, and there is something we need to investigate further (i.e. there could be a bug)

It'd be great if you could get back to us on this, and we can help get your system working as it should.

Simon

Yes, I agree the first code is an oversight, there's no way the IMU would respond in that time.  In the second case, I would agree on the condition that the loop goes faster than the IMU can write its entire response packet (which, I imagine, is likely).

The reason I wanted to use this test is because read will block until there is a packet available and I don't want the program to crash in the event of a write error.  My thought on the behavior of readable() was simply that it returns whether or not there is a byte on the rx buffer.  Now I'm trying to verify my understanding so that I can write error safe code.

I also tried putting a wait_ms(20); after the command is sent to allow enough time for the IMU to respond before I call readable(), however this broke the code; I was getting zero activity on both the IMU and the Computer ports.  The next idea I'm thinking about trying is inserting

while(!test)
{
    test = IMU.readable();
}

before the reading takes place.

 

After I verify the method works, I think the idea is to put a timeout in the code for the IMU response.