Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to port "readable" #59

Open
sagonzal opened this issue Nov 14, 2016 · 18 comments
Open

How to port "readable" #59

sagonzal opened this issue Nov 14, 2016 · 18 comments

Comments

@sagonzal
Copy link

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
Copy link
Owner

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
Copy link
Author

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

@Overdrivr
Copy link
Owner

Overdrivr commented Nov 14, 2016

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

@sagonzal
Copy link
Author

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

@Overdrivr
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

Overdrivr commented Nov 16, 2016

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

  • 3 more bytes for the topic (2 characters + end of string),
  • 4 bytes containing the payload (only one is used for the data with a uint8)
  • 2 bytes for the CRC
  • and end with 0x7F

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:

  • Watch out received characters and see if they follow the pattern. Otherwise, you might have an issue with the uart driver not buffering properly
  • Call publish_u8("Kp", uc_control); in your loop, see if the value changes
  • Set a breakpoint here and see if it triggers. It will if a complete frame was decoded successfully

@sagonzal
Copy link
Author

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
Copy link
Author

sagonzal commented Nov 18, 2016

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
Copy link
Owner

Overdrivr commented Nov 18, 2016

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)

  • Which OS and architecture are you running on ?
  • Which version of pytelemetry and pytelemetrycli do you have ?

I will investigate on my side also

Thanks

@Overdrivr
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

sagonzal commented Nov 18, 2016

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

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:

  • as you know I ported telemetry (plain C) to the K22FN512 (a cortex M4 cpu) but I never told you that I was running a complex control algorithm with I2C peripheral in the background. With heavy processing.

  • as you also know, I modified your driver.h so 'readable' and 'writable' are now different.

  • I'am using a secondary (not resource full) USART and they do NOT have a buffer.

  • the problem was that update_telemetry() was called within a interrupt and that can not happen if you desire telemetry to treat a transmission byte by byte since it collapses withe the reception. If I only send a single packet, it was more or less OK, but since I sent constantly to the host, the host hasn't the chance of sending anything!.

  • So with careful timing and update_telemetry() in the main loop, I solved the issue but only when low length packet are sent to the embedded device.

  • I should rewrite writable to place a semaphore or check the usability of the USART as I now did on readable

--> 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants