hoverboard-firmware-hack-FOC icon indicating copy to clipboard operation
hoverboard-firmware-hack-FOC copied to clipboard

Bipropellant protocol

Open p-h-a-i-l opened this issue 5 years ago • 15 comments

Work in progress. already tested and working: USART3 PWM control over bipropellant protocol

p-h-a-i-l avatar Jun 26 '20 18:06 p-h-a-i-l

Awesome that you are working on it! I still have to look a bit in more detail over the code.

At a quick look, I have one remark. Instead of interrupt based UART can we use the DMA UART that I made, with circular buffer and Idle Line interrupt? You can check the framework in util.c It works like a charm on my hovercar, not a single timeout. So, it would be a pity not to use it. Plus it also gives consistency in the firmware.

EFeru avatar Jun 27 '20 07:06 EFeru

It is a work in progress, not everything implemented and some parts do nothing which need to be removed. I'd rather use DMA based UART for sure, but unfortunately I don't know how to get messages with variable length. Do you have an idea? Sending should be rather simple, for receiving I need something which feeds protocol_byte() with received bytes to process

p-h-a-i-l avatar Jun 27 '20 11:06 p-h-a-i-l

Yes, I did that already for the Serial Debug. If you look at this function in util.c https://github.com/EmanuelFeru/hoverboard-firmware-hack-FOC/blob/d8b529e063b1a03908cd1508e05af5b329eb348b/Src/util.c#L818-L841 And below Is the processing done byte by byte. The len of the received data can vary. https://github.com/EmanuelFeru/hoverboard-firmware-hack-FOC/blob/d8b529e063b1a03908cd1508e05af5b329eb348b/Src/util.c#L957-L970

Of course, you can create you own custom usart_process_data(...) according to your needs. What processing do you need to do?

EFeru avatar Jun 27 '20 13:06 EFeru

very nice! Changed the implementation now to DMA USART.

p-h-a-i-l avatar Jun 28 '20 15:06 p-h-a-i-l

2 more remarks :)

  • What is the scope of SERIAL_USART2_DMA and SERIAL_USART3_DMA? Can we give a more suggestive name? maybe... PROTOCOL_SERIAL_USART2 and PROTOCOL_SERIAL_USART3, depends for what they are used for. And for consistency, in general is nice if we keep ..._USART2 , ..._USART3 at the end of the name, to match the rest of the Serial defines.
  • and I have seen in some places (maybe you removed them already) pragma once. Apparently, Keil is more restrictive and doesn't like it. So, can we replace them with:
// Define to prevent recursive inclusion
#ifndef zzz_H
#define zzz_H
.....
#endif // zzz_H

EFeru avatar Jun 28 '20 16:06 EFeru

Two easy points to change. Btw, you should also be able to commit to my branch if you like to contribute. Right now I think the use of ifdefs is a bit excessive, I'd like to clean up a little, but it is not a priority.

p-h-a-i-l avatar Jun 28 '20 22:06 p-h-a-i-l

@EmanuelFeru: Can you have a look at my usage of the sending routine? I guess I'm doing something wrong. Receiving is working well, but sending is very unstable..

I guess the problem is in USART3_DMA_send()..

You are using: if(__HAL_DMA_GET_COUNTER(huart3.hdmatx) == 0) { HAL_UART_Transmit_DMA(&huart3, (uint8_t *)uart_buf, strLength); } What is are you doing with the Counter? Do I need to implement a buffer?

p-h-a-i-l avatar Jun 29 '20 16:06 p-h-a-i-l

The check below has to be there to make sure the DMA counter (Counts down) has reached 0. This indicates that previous data has been sent.

if(__HAL_DMA_GET_COUNTER(huart3.hdmatx) == 0)

Now, if you try to send data faster than it can be physically sent, then not all your data will be send, because the if check will not pass all the time. In this case buffering is needed, to put all data in a buffer and load the DMA once. But if you make sure to have some time in between the sends, then buffering is not needed. Try to see how it behaves with the if condition and check that you are not calling the send function one after each other (without any time in between).

Are you sending ASCII data? I am not sure if it will make a difference, but I compute the length using strlen((char *)(uintptr_t)data) and it works. It might work also with sizeof(data), but I haven't played with it.

EFeru avatar Jun 29 '20 18:06 EFeru

Something is very fishy.. Indeed __HAL_DMA_GET_COUNTER is not ready yet. But when I wait for it to clear, way too much time goes by. Also looks like some Messages are sent too short.

p-h-a-i-l avatar Jun 29 '20 21:06 p-h-a-i-l

Ok, I guess I know whats happening.. I need a buffer. Just sending garbage from reused memory locations

p-h-a-i-l avatar Jun 29 '20 21:06 p-h-a-i-l

Ok, with a buffer the garbage problem is solved. Still the delay is huge. I have a test setup running a simple ping command. Usually I can see a ping of 14ms (protocol relayed via Wifi to UART) when using interrupt based UART, witth DMA it is around 500ms to 700ms

p-h-a-i-l avatar Jun 29 '20 21:06 p-h-a-i-l

Hmm, that is strange, because I do send data to my Sideboard every ~10 ms depending on the main loop duration (see below). If I have time I will quickly check with an oscilloscope if data is indeed sent at ~10ms intervals. https://github.com/EmanuelFeru/hoverboard-firmware-hack-FOC/blob/d8b529e063b1a03908cd1508e05af5b329eb348b/Src/main.c#L413-L443

EFeru avatar Jun 30 '20 12:06 EFeru

Apparently, I created a conflict in util.cwith my last commit, but I don't see the conflict... do you know how to fix it?

EFeru avatar Jul 01 '20 17:07 EFeru

Hi @p-h-a-i-l , First of all happy new year! What is you plan with this PR, intend to continue on it?

EFeru avatar Jan 01 '22 11:01 EFeru

Hi, is this pr working in the current state? Or is there still some things that need to be fixed?

Rubiaceae65 avatar Aug 10 '22 16:08 Rubiaceae65