lexus2k / tinyproto

Tiny Software Protocol for communication over UART, SPI, etc
GNU General Public License v3.0
225 stars 51 forks source link

Will Not Compile on Cortex M0 #2

Closed NickWaterton closed 8 years ago

NickWaterton commented 8 years ago

src/TinyProtocol.h:266:73: error: call of overloaded 'put(int)' is ambiguous

I'm trying to implement reliable serial communications between two Cortex MO's using Serial 1.

I'm assuming I'm getting this error because of the 32 bit platform. I'm not sure where the ambiguity is between the various forms of put though.

Any suggestions?

lexus2k commented 8 years ago

Thank you very much for the finding. I fixed that issue. I don't have ARM near by my hand, but I tested and fixed compilation on x86 32-bit platform. Please, download latest package and let me know if it works now for you.

NickWaterton commented 8 years ago

Wow, that was quick!

I'll try it tomorrow.

Thanks,

NickWaterton commented 8 years ago

Ok,

Tested new version. I'm using the Arduino code.

The echo example now compiles, but does not appear to do anything. ie I get no echo of anything typed into the serial monitor.

The sketch_tinyProto01 does not compile, but it's a simple conflict between this line

const int LED_PIN = 13;

and a define somewhere else in code. commenting out this line enables it to compile.

Having done that, I'm not sure what this example is supposed to show, now I just get continual output of the character ~ in the serial monitor.

I'm having a hard time understanding how the protocol is declared or used (eg is char g_buf[16]; supposed to be the payload data? or some sort of internal header?

I'm sending data that ranges from a few bytes up to a max of (4+(1400*3)) in size. should I declare a buffer of a maximum of this size? even if I only need to send a few bytes? or SERIAL_BUFFER_SIZE (normally 64 bytes) which is what I'm doing right now. I am sending data that is mostly compressed, but could also be raw, hence the huge size variation. The raw is the standard data type (which I cannot change as it's a published protocol), I've added my own custom data type to the protocol, that uses compressed data to reduce this to a more manageable size.

I'm building a Serial to UART bridge, eventually a WiFi to UART bridge, and I'm having a hard time translating the usual Serial.readBytes(buf, size) Serial.write(buf, size), Serial.available() (or client for WiFi) functions to equivalents for your protocol.

Can you give me some pointers?

I have working code without any protocol being used (just direct Serial to Serial1, Serial1 to Serial), but it's very finicky on timing, and if I drop a byte anywhere, it loses sync. The data is very basic, just 4 bytes header (includes size of payload), then the payload with no checksum or crc, so if it gets out of sync, it starts reading data as the the payload size, then it all goes wrong. I'd like to wrap the data in a protocol so that I can transport it intact (or know when it's not intact).

Any help is appreciated.

NickWaterton commented 8 years ago

Looking through your code, I think I've found one problem (which I think is my problem not yours).

The BeginToSerial() function:

    /**
     * Initializes protocol internal variables and redirects
     * communication through Arduino Serial connection (Serial).
     * @return None
     */
    inline void beginToSerial()
    {
         begin([](const uint8_t *b, int s)->int { return Serial.write(b, s); },
               [](uint8_t *b, int s)->int { return Serial.readBytes(b, s); });
    }

Does not work in my code, as the Serial object is not a UART but a virtual USB port (sort of Leonardo style), which I have found in my experiments does not return the expected value for Serial.write. This should be the number of bytes written, but in my case it returns 0 every time.

Serial.readBytes() works as expected, and Serial.write() to an actual UART does return the expected values. So I think this is a quirk (bug) of the M0 board specific code. I'll have to dig into the board code to see what it's doing, as it's killing me that I can't read how many bytes were actually sent (right now I'm just assuming that it all went).

I'll let you know if I figure it out.

Regards,

lexus2k commented 8 years ago

PIN_LED isn't reserved word and is defined only once in the sketch. At least I didn't find anything over internet about that keyword. I have only ATmega368 controller to test and TI LM242F, which is ARM 32-bit, but I don't use C++ code for it, only C-style API instead. So, all these errors are about Arduino wrapper. Originally this library was developed for non-Arduino systems with C-style API.

As for examples, echo_example is intended to send all packets received back to UART port. But you cannot simply send 'AB', you should place all data, you send, with ~ chars around: '~AB~'. And then Arduino will send that data back to UART.

sketch_tinyProto01 sends ~Hello~ all the time, and if you send some packet to it, it will echo the packet back.

To reduce memory consumption, the protocol itself doesn't have any buffers inside. It assumes that a user will allocate the buffer to fit all data, being sent, or received. Again, Tiny::Packet is just wrapper, which helps to work with the allocated buffer (g_buf in my examples). You can put data to g_buf directly, not using Tiny::Packet class.

I'm sending data that ranges from a few bytes up to a max of (4+(1400*3)) in size

If you want to send up to 1400*3 + 4 = 4204 bytes in single packet, then you need to allocated the buffer of that size in your program (and I recommend to allocate slightly larger). Next, if you want to send and received packets in non-blocking mode (asynchronous), you need to define 2 buffers: for the packets being sent and the packets being received.

char g_sendBuf[200];
char g_receiveBuf[4210];

These 2 buffers will be used by Tiny protocol to accumulate data being received until it recognize the whole Packet, and the data being sent until it says you that the Packet is completely sent.

even if I only need to send a few bytes? or SERIAL_BUFFER_SIZE (normally 64 bytes) which is what I'm doing right now.

The main purpose of this protocol is to detect errors on UART line (or on other physical line). You cannot figure out if the packet is broken or not until it received completely along with FCS field. So, if you what to send large packets, yes, you need to declare the buffer of maximum size.

The raw is the standard data type (which I cannot change as it's a published protocol), I've added my own custom data type to the protocol, that uses compressed data to reduce this to a more manageable size.

What is this published protocol?

I'm building a Serial to UART bridge, eventually a WiFi to UART bridge, and I'm having a hard time translating the usual Serial.readBytes(buf, size) Serial.write(buf, size), Serial.available() (or client for WiFi) functions to equivalents for your protocol.

Ok, so, you have 2 sides on UART line. Do you use microcontroller or some sort of CPU? How much memory do you have? Why do you need translation of usual readBytes(), write() to protocol functions? Actually the protocol is intended to read/write anything to Serial by itself. And of course, the Tiny::Proto should be compiled and used on both sides on UART.

I have working code without any protocol being used (just direct Serial to Serial1, Serial1 to Serial), but it's very finicky on timing, and if I drop a byte anywhere, it loses sync. The data is very basic, just 4 bytes header (includes size of payload), then the payload with no checksum or crc, so if it gets out of sync, it starts reading data as the the payload size, then it all goes wrong. I'd like to wrap the data in a protocol so that I can transport it intact (or know when it's not intact).

OK, if you need to transport some data over UART/Serial line, can you split them if they are too big to fit in memory at once? Next, actually you don't need to convert, I mean, the converting is very simple:

char receiveBuf[1024];
Tiny::Packet inPacket(receiveBuf, sizeof(receiveBuf));
void DoSerialRead()
{
    int len = proto.read(inPacket, TINY_FLAG_WAIT_FOREVER);
    if (len > 0)
   {
         // You can refer to receiveBuf directly here to work with your data
        // or use inPacket methods to access the data
   }
}
char sendBuf[1024];
Tiny::Packet inPacket(sendBuf, sizeof(sendBuf));
void DoSerialSend()
{
    // Put you data to send directly to the sendBuf
    proto.write(sendBuf, your_data_size, TINY_FLAG_WAIT_FOREVER);
}

That's all conversion. The Tiny protocol doesn't require you to use Tiny::Packet class. You can you char buffers. But just don't forget to enable CRC, you want to use.

Does not work in my code, as the Serial object is not a UART but a virtual USB port (sort of Leonardo style), which I have found in my experiments does not return the expected value for Serial.write. This should be the number of bytes written, but in my case it returns 0 every time.

If you're sure, that your Serial.write actually sends byte (but returns 0), you can write your own callbacks:

proto.begin([](const uint8_t *b, int s)->int { Serial.write(b, s); return s; },
                 [](uint8_t *b, int s)->int { return Serial.readBytes(b, s); });

But you should be carefull here. What if Serial.write will not write anything?

NickWaterton commented 8 years ago

Thanks for the pointers,

The LED_PIN issue is that it is defined elsewhere in the Arduino code as

define LED_PIN 13

So the compiler won't allow you to redefine it as const int. Not a big deal.

The protocol being used is called OPC for Open Pixel Control see http://openpixelcontrol.org/

I have two microcontrollers with cortex M0 CPU's, each has 32k SRAM, one has a WiFi module also. One is the pixel controller, the other is the communication controller. The pixel controller already has large buffers declared to hold the state of the pixels (3 of them, which it cycles) and two large SPI buffers for DMA transfer to the pixel array. Consequently it only has 700 bytes of Ram left.

The communications controller has WiFi, and is being used for sensors etc. The plan is for it to receive data over WiFi or USB (for testing) then transfer data via UART to the pixel controller. I can run the UARTs at 2Mbaud with no problems. The communications controller has 20k Ram free at the moment, and one large (4204 bytes) buffer defined.

What I need to do is receive one buffer (up to 4204 bytes) via WiFi or USB (which is reliable), then send this buffer to the pixel controller via UART, which has a similar 4204 byte buffer waiting to be filled. The issue is that if I get out of sync on the UART transfer, there is currently no detection or control on this transfer (unlike USB or WiFi which have their own built in error correction). The UART ring buffer is only 64 bytes, so I've been using 32 byte packets, as I don't seem to be able to send 64 bytes at a time reliably.

So I already have the buffers defined, and I only need to implement a protocol on Serial 1 (the UART). But I need something small, as my pixel controller is almost out of SRAM. Communications controller has plenty of SRAM. Flash is not a problem.

Your protocol looked like the answer, so I want to try it out. I have some test code rigged up that sends via USB to the communications controller, via UART to the pixel controller, then USB back to the source, which compares the two. This I can do with no issues at 2Mbaud. The problem comes when I try this for real, the pixel controller is doing a lot of other things, and looses sync from time to time. I need a way to detect this, and resync.

I will try you examples on my test code, to see how it works.

Thank you for your help.

lexus2k commented 8 years ago

The plan is for it to receive data over WiFi or USB (for testing) then transfer data via UART to the pixel controller. I can run the UARTs at 2Mbaud with no problems. The communications controller has 20k Ram free at the moment, and one large (4204 bytes) buffer defined.

You mentioned earlier that you have several buffers. So, when one buffer is ready for sending, you can use it to transfer data over UART using proto.write(buf, len, TINY_FLAG_WAIT_FOREVER);

If you need to have "parallel processing" (because UART speed is still slow), you can use TINY_FLAG_NO_WAIT, but you need to check that proto.write returned number of bytes sent greater than zero. If it returns zero, just call it with the same params next time. I mean call proto.write(buf, size, TINY_FLAG_NO_WAIT) until it returns positive value greater than zero.

Tiny protocol respects uC resources, so, you can reuse already defined buffers with this protocol.

lexus2k commented 8 years ago

And one more thing. if you want to detect broken packets enable CRC in the protocol, at least simple checksum: proto.enableCheckSum( );

NickWaterton commented 8 years ago

Thanks for all your help with this. I have it working now. I'm sending my buffers 64 bytes at a time, as that's the serial buffer size.

The problem I have now, is that now I can detect the bad packets, I'm seeing an error rate of 13%, and baud rate does not make a difference - I've tried it from 3Mbaud to 9600 baud, and the error rate stays the same.

I knew it was dropping bytes, but 13% ! That's crazy.

I need to figure out what is going on here, as I can't live with this error rate...

Thanks again.

lexus2k commented 8 years ago

13%... That's a lot. UART itself usually doesn't give such a big error rate. In my tests I never saw UART error at 3Mbaud. If the code has single thread only, maybe, the bytes are lost during processing some other things.

I'm confused that an error rate is the same for different baud values. Let me know, if bytes are lost in the protocol.

NickWaterton commented 8 years ago

I think I have a handle on it. It’s not losing bytes in your protocol, it happens with or without the tiny protocol (I can just measure it easier with).

I said it was finicky on timing, it’s something to do with the M0 board code. There is a difference of a few bytes between what Serial1.available() reports and what you can actually read (or what is reported as read). – Only for a few ms, then it “catches up”, but that is enough to screw it up in a tight timing loop.

If you put a Serial.print in the loop to see what is actually going on, then everything works fine – the Serial.print slows the loop down enough, that the reported vs actual catches up with itself (but kills the baud rate).

I have a workaround in place running at 3MBaud with no errors currently. Ran it for 3 hours today – 0 errors.

I also swapped in a cortex M3 (running at 120 MHz), which does not have this problem (works as is), and that runs without any errors at 3MBaud, with no work-arounds. It also reports serial.write correctly on the usb port.

So at this point I’m considering using the Cortex M3 as the communications controller, and ditching the M0 (the M3 has WiFi also). I’m still stuck with the M0 for the pixel controller, as the M3 only has 16K of SRAM, and I need at least 32k.

I don’t think you need worry about your code, the Arduino zero/M0 on the other hand has some serious bugs.

Nick Waterton P.Eng. National Support Leader - Nuclear Medicine, PET and RP GE Healthcare

M+1 416 859 8545 F +1 905 248 3089 E nick.waterton@med.ge.commailto:nick.waterton@med.ge.com

2300 Meadowvale Boulevard Mississauga, ON L5N 5P9 Canada

GE imagination at work

From: lexus2k [mailto:notifications@github.com] Sent: April-26-16 6:51 PM To: lexus2k/tinyproto Cc: Waterton, Nick (GE Healthcare); Author Subject: Re: [lexus2k/tinyproto] Will Not Compile on Cortex M0 (#2)

13%... That's a lot. UART itself usually doesn't give such a big error rate. In my tests I never saw UART error at 3Mbaud. If the code has single thread only, maybe, the bytes are lost during processing some other things.

I'm confused that an error rate is the same for different baud values. Let me know, if bytes are lost in the protocol.

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_lexus2k_tinyproto_issues_2-23issuecomment-2D214911662&d=CwMCaQ&c=IV_clAzoPDE253xZdHuilRgztyhRiV3wUrLrDQYWSI&r=U-uBTyQDj4ifd00J7nSYtwgRmrVu2vacy9QbRAMDQ&m=c-DznZOuL3ITXcuRdi1vImMJ30pNQ8dY-zy2PRwgjs&s=4wr9mAllTbltBt6IUF5Qi81iP9vCfAmogUieQZDMRzE&e=

Unsubscribe from our commercial electronic messages. http://supportcentral.ge.com/esurvey/takesurvey.asp?p=17778&d=3834102 Désabonner de nos messages électroniques commerciaux. http://supportcentral.ge.com/esurvey/takesurvey.asp?p=17778&d=3839563

lexus2k commented 8 years ago

Thank you for the comments.