Overdrivr / Telemetry

Data visualization and communication with embedded devices
MIT License
136 stars 27 forks source link

How to port "readable" #59

Open sagonzal opened 7 years ago

sagonzal commented 7 years ago

First Rémi, thank you for your hard work. I ported telemetry (plain C) to the K22FN512 (a cortex M4 cpu) platform on the K22F freedom board. And only recently I'm in the need of sending data from the host to the embedded.

I'm facing the issue that my variable is not updated even thou that I use update_telemetry(). I'm sure the issue is with:

void update_telemetry()
{
  // If user forgot to define transport by calling init_telemetry, abort
  if(!transportPtr)
    return;

  uint32_t amount = transportPtr->readable();
  uint32_t i = 0 ;
  for(i = 0 ; i < amount ; i++)
  {
    uint8_t c;
    transportPtr->read(&c,1);
    feed(c);
  }
}

The issue is that I don't have any "serial.readable();" And place the following in the driver:

*/ int32_t readable() {
    uint32_t amount = 0;

    return amount;
}

Now my question is, amount has to be a single 8 bit char (amount = 1)or a whole message (amount = N)? That is the main difficult to porting since I do not know wow the real function "readable" works..

warmest regards

Overdrivr commented 7 years ago

Hi sagonzal, thank you for the kind words.

readable should return the amount of 8-bit characters in the UART reception buffer of your k22F.

Look for instance how it is done for the [arduino](https://github.com/Overdrivr/Telemetry-arduino/blob/master/src/transport.h]

The exact function you need to call to get this amount depends on the UART library you are using (or maybe coded yourself) for the K22F.

Is this helpful ?

sagonzal commented 7 years ago

OK, so you say to use the hardware FIFO and readable returns the chars in the hardware FIFO, right?

Overdrivr commented 7 years ago

No, readable should return just the amount of characters in the FIFO. It's read function that returns the characters themselves

sagonzal commented 7 years ago

Would it be all right if it only returned 1, when a single byte is in the hardware FIFO ?

Overdrivr commented 7 years ago

Nope, it's fine :) It will just read a single byte every time update_telemetry is called, instead of all characters in the FIFO.

It will just take more time for your incoming commands to be decoded as you might have guessed.

The only issue you can face is in case you send commands too quickly, the FIFO may be written faster than read, and characters can be overwritten, causing decoding issues. Appart from that, no problem

Overdrivr commented 7 years ago

Actually, on the arduino code, this is already what I have done, since it's easier to fetch characters from the FIFO one at a time (no arrays to deal with)

sagonzal commented 7 years ago

Sadly, it is still not working. Help me going over the steps requiring sending a byte from host to the embedded:

First on the start up of the embedded routine, what I do is:

    // publish some variables
    publish_f32(g_duty_L, duties[0]);
    publish_f32(error, duties[2]);

    // Subscribe uc_control variable to topic "Kp"
    attach_u8("Kp", &uc_control);

    update_telemetry(0);

Then, every time I go to a routine in the main loop I do:

            attach_u8("Kp", &uc_control);

            update_telemetry(0);

in order to bring to the embedded the updated variable, uc_control, in this case.

Then on the pyTelemetryCLI I issue:

:> pub --u8 Kp 1 Published on topic 'Kp' : 1 [uint8]

in order to publish to the embedded.

In the embedded the 'update_telemetry' routine receives the characters. Two characters are in fact, a preamble byte like 0xF0 and the 0x01. But then 'amount' is always 1 and the routine goes to 'feed()' and nothing happens since transportPtr->read(&c,1) in update_telemetry().

What I'm doing wrong ?

Overdrivr commented 7 years ago

First, you seem to be using syntax from different versions of telemetry. The functions attach_XX are from 2.0.0, but in this version update_telemetry no longer requires any parameter. Yet you seem to be able to call it with update_telemetry(0);. Maybe you only get a compiler warning, otherwise you should not be able to pass this 0 as paremeter. Could you check if your version of the code is 2.0.0 and not another ?

attach_u8("Kp", &uc_control); needs only to be called during initialization, it keeps tracks of variables afterwards.

How many characters do you receive in total ?

The frame should start by 0xF7 Then be followed by a two byte indicator, here equal to 0x0001 indicating a uint8_t payload.

Afterward you should receive

You could get 1~2 byte extra for escaping it the frame contains 0x7D somewhere, although pretty unlikely for such a small frame.

So, to debug your problem, you can:

sagonzal commented 7 years ago

Dear Rémi, version is

define TELEMETRY_VERSION_MAJOR 2

define TELEMETRY_VERSION_MINOR 0

define TELEMETRY_VERSION_PATCH 0

so, yes, 2.0.0

Yes i get a warning message and I can call the function update_telemetry(), no problems in that side. And the framing stars OK and then it comes the 0x01 but the breakpoints it may be breaking the code flow in the framing. Allow me to try this. Thank you in advance.

sagonzal commented 7 years ago

Dear Rémi, unfortunately I still have a several byte arriving staring with is 0xF7.

But the main issue, I think, is that EOF is not arriving at all:

Let me show you the stats command before two writes and after it:

:> stats Raw IO: tx_chunks : 0 rx_chunks : 339917 rx_in_waiting_max : 289 rx_in_waiting : 4 rx_bytes : 322950 rx_in_waiting_avg : 2.264745024795975 tx_bytes : 0 IO speeds: baudratio_avg : 0.08463095014577876 baudspeed : 9839.361918820878 baudratio : 0.08541112776754234 baudspeed_avg : 9749.485456793713 Framing: rx_discarded_bytes : 183718 rx_escaped_bytes : 419 tx_escaped_bytes : 0 rx_uncomplete_frames : 1043 rx_processed_bytes : 322958 tx_processed_bytes : 0 tx_encoded_frames : 0 rx_complete_frames : 9048 Protocol: rx_decoded_frames : 6379 rx_corrupted_topic : 0 rx_corrupted_header : 15 rx_corrupted_payload : 0 tx_encoded_frames : 0 rx_corrupted_eol : 1 rx_corrupted_crc : 2026 :> pub --u8 Kp 1 Published on topic 'Kp' : 1 [uint8] :> pub --u8 Kp 1 Published on topic 'Kp' : 1 [uint8] :> stats Raw IO: tx_chunks : 2 rx_chunks : 581867 rx_in_waiting_max : 289 rx_in_waiting : 7 rx_bytes : 533264 rx_in_waiting_avg : 6.137334444449687 tx_bytes : 20 IO speeds: baudratio_avg : 0.08466988110364666 baudspeed : 9535.381631897955 baudratio : 0.0827724099991142 baudspeed_avg : 9753.970303140095 Framing: rx_discarded_bytes : 323855 rx_escaped_bytes : 631 tx_escaped_bytes : 0 rx_uncomplete_frames : 2967 rx_processed_bytes : 533270 tx_processed_bytes : 16 tx_encoded_frames : 2 rx_complete_frames : 13735 Protocol: rx_decoded_frames : 6506 rx_corrupted_topic : 0 rx_corrupted_header : 46 rx_corrupted_payload : 0 tx_encoded_frames : 2 rx_corrupted_eol : 9 rx_corrupted_crc : 5413 :>

As you can see it seems to send 20 bytes, and not 22 could it be the CLI ?

Overdrivr commented 7 years ago

It's actually worse than that, because two frames were sent, so I was expecting tx_bytes=44.

Could you open the generated logs folder (created where you started the pytelemetrycli) and share the file ./logs/2016-Nov-DD_hh-mm-ss/out-2016-Nov-DD_hh-mm-ss.txt ? (replace with corresponding date) It contains all the frames sent by the CLI so we will know a bit more what is going on. (This file contains the entire frames except the SOF and EOF)

I will investigate on my side also

Thanks

Overdrivr commented 7 years ago

Sorry, made a mistake when describing the protocol. For uint8, it only uses a single for the payload.

So the complete frame should be

1 (SOF) + 2 (indicator) + 3 (topic) + 1 (payload) + 2 (CRC) + 1 (EOF) = 10 bytes

Which is consistent with your log data (tx_bytes is the count of absolutely all characters).

I have checked, both pytelemetry and Telemetry return the same frame with those parameters: (hex format)

f7 0100 4b7000 01 0926 7f

So in the log, you should see

0100 4b7000 01 0926

If you can confirm it, it most likely means the issue is on the other side. Could you place a breakpoint herefor instance and report the bytes you see there ?

Thanks a lot

sagonzal commented 7 years ago

Let me see, on the 'out' log:

2016-11-18 10:31:46,698 | INFO | b'01004b7000010926' 2016-11-18 10:32:00,683 | INFO | b'01004b7000010926' 2016-11-18 10:32:40,168 | INFO | b'01004b7000010926' 2016-11-18 10:38:00,145 | INFO | b'01004b7000010926' 2016-11-18 10:38:05,770 | INFO | b'01004b7000010926' 2016-11-18 10:38:06,364 | INFO | b'01004b7000010926' 2016-11-18 10:39:12,537 | INFO | b'01004b7000010926' 2016-11-18 10:39:44,787 | INFO | b'01004b7000010926' 2016-11-18 10:41:10,398 | INFO | b'01004b7000010926' 2016-11-18 10:41:30,493 | INFO | b'01004b7000010926' 2016-11-18 10:41:32,274 | INFO | b'01004b7000010926' 2016-11-18 10:42:49,197 | INFO | b'01004b7000010926' 2016-11-18 10:57:15,974 | INFO | b'01004b7000010926' 2016-11-18 10:57:28,287 | INFO | b'01004b7000010926' 2016-11-18 10:58:15,585 | INFO | b'01004b7000010926' 2016-11-18 10:58:17,491 | INFO | b'01004b7000010926' 2016-11-18 10:59:18,805 | INFO | b'01004b7000010926' 2016-11-18 10:59:25,743 | INFO | b'01004b7000010926' 2016-11-18 10:59:38,774 | INFO | b'01004b7000010926' 2016-11-18 10:59:57,743 | INFO | b'01004b7000010926' 2016-11-18 11:00:02,009 | INFO | b'01004b7000010926' 2016-11-18 11:00:02,790 | INFO | b'01004b7000010926' 2016-11-18 11:01:34,980 | INFO | b'01004b7000010926' 2016-11-18 11:01:36,621 | INFO | b'01004b7000010926' 2016-11-18 11:02:23,545 | INFO | b'01004b7000010926' 2016-11-18 12:39:39,692 | INFO | b'01004b7000010926' 2016-11-18 12:39:44,817 | INFO | b'01004b7000010926' 2016-11-18 12:39:45,552 | INFO | b'01004b7000010926' 2016-11-18 12:39:45,786 | INFO | b'01004b7000010926' 2016-11-18 12:39:45,974 | INFO | b'01004b7000010926' 2016-11-18 12:39:46,161 | INFO | b'01004b7000010926' 2016-11-18 12:39:46,349 | INFO | b'01004b7000010926'

...so it clear that it is sending, right ?

Now the breakpoint, should it be on line 131 ? since it is after a return.

Overdrivr commented 7 years ago

You can place it on line 129, although it should not enter the if condition unless there was some memory allocation issue, which is unlikely.

Yes the data has been sent correctly, so it's most likely due to the microcontroller. Could you post the contents of transport.h ? And eventually your UART library ?

sagonzal commented 7 years ago

Sorry Rémi, but I do not have any transport.h and driver.h is just almost a wrapper to point to my actual uart functions. The funny thing about this is that I start receiving OK but somehow the message is broken somewhere. I'm starting to think this is a timing issue since you don't process the whole message but you prefer to feed it a byte a t a time.

In hat case, in update_telemetry():

uint32_t amount = transportPtr->readable(); uint32_t i = 0 ; for(i = 0 ; i < amount ; i++) { uint8_t c; transportPtr->read(&c,1); feed(c); }

Why you chose to accept 'amount' to be any byte number but you force read(&c,1) reading byte by bye ?

Overdrivr commented 7 years ago

The RX buffer is read 1 by 1 because the decoding function works on 1 byte at a time. This is required by the escaping algorithm for frame delimiting. But I kept the read(ptr, amount) interface because it is pretty much the standard for UART interfaces.

If you believe there is a timing issue, then it's related to the UART driver not buffering enough received characters. Usually, default hardware drivers will buffer very few characters, and a larger software buffer must be implemented almost always. This is done on arduino, on mbed, etc. This was also the case for the Kinetis KL25Z I used for most testings.

The buffering is not done by Telemetry because first I want to keep RAM consumption as low as possible, and because like I said it is sometimes already done by UART drivers.

Could you check if there is any software buffering inside your driver ? Like a 64~128 byte buffer on the RX.

I should make it clearer somewhere in the docs that the UART driver should buffer for proper operation.

By the way, now that I am looking at the stats you provided, I am almost sure there is no buffering either on the TX side, because I see that you have almost as many corrupted frames as valid ones.

PS: Hope this is not too frustrating, sorry I cannot be of more efficient help, but those UART drivers can be some tricky beasts sometimes

Overdrivr commented 7 years ago

Hi sagonzal, I was wondering if you were able to solve your problemsm, if you need some help finding the buffered serial lib I indicated, I can give you a hand.

I'm leaving this issue open for the moment

sagonzal commented 7 years ago

Rémi, huge thanks for your support here. Let me tell you that I solve the issue and (as always are) was a complex combination of factors:

--> more on this as soon as I have some time to delve into this again. I strongly feel that a multi-byte buffer can help here.