ttlappalainen / NMEA2000

NMEA2000 library for Arduino
538 stars 226 forks source link

STM32F103C8 Bluepill new library #94

Open jiauka opened 6 years ago

jiauka commented 6 years ago

Hi, thanks a lot for your project.

I have done a CAN library that uses the ST official arduino port for the stm32.

https://github.com/stm32duino The library is at https://github.com/jiauka/NMEA2000_stm32f1

To use it, the NMEA2000_CAN.h needs some tweak.

elif defined(STM32F103xB)||defined(STM32F103xB)

define USE_N2K_CAN USE_N2K_STM32F1xx_CAN

elif USE_N2K_CAN == USE_N2K_STM32F1xx_CAN

include

tNMEA2000 &NMEA2000=*(new NMEA2000_stm32f1());

Thanks again!! j.

ttlappalainen commented 6 years ago

Thanks.

Some notes.

In principle you can sent all by waiting, but that takes performance from other system tasks.

jiauka commented 6 years ago

Thanks a lot!

MIT license, ok. wait and tx buffers. I know and it is a WIP.

I didn't know about priority, I need to work oit a solution, I guess it's not easy to implememt with current stm32 libs.

yours!

j.

ttlappalainen commented 6 years ago

It is common problem with all CAN systems, since they do not implement NMEA 2000 fast packet. That is why I had to add functionality to mcp_can, FlexCan and due_can. FlexCan comes with Teensy but they have not accepted my improvements. Either improved mcp_can is not under Seed Studio library, which I think is most common. Also other developer added my library to MBED, but also their CAN driver does not have right handling for fast packet, so it is useless and MBED people has not been interested about problem.

I see that only solution would be to forl stm32 library and develop it. You can look my developed FlexCan version how I solved the problem. Then you could try to push improvements to official version. Other option would be to skip lover level and write also lower level handling to the NMEA2000_stm32f1 module.

jiauka commented 6 years ago

Thanks for your comments.

I your opinion, which one of the lower can libs is better to have a look at for the fast packet implementation, mcp_can, FlexCan or due_can?

Which is the best way to check fastpacket functionality?

Forking the library is a no go, there are 10!!! HAL drivers for the F0, F1, F3... CPUs. Skip the lower level and include it at the NMEA2000_stm32f1 is also a pitty, it will work only for one family.

Once I have it working, I will contact Arduino official STM32 maintainer, this is by far the best solution.

The lib needs some code cleaning, TX interrupt implementation, pin conf selection and some minor improvements, I will do them in the next days, and try to implement the fastpacket stuff is the most neat way.

yours,

j.

jiauka commented 6 years ago

Hi:

I have been looking at the fastpacket stuff and I think I fully know the problem, unfortunately stm32 libs are not suitable, but looks not difficult to modify them and the maintainer is willing to accept a PR.

But I have some questions, looking at your NMEA2000_teensy, there is something I don't correctly understand, see my comments at the source code below (rewritten to split the conditional operator)

` if ( wait_sent ) { // Use last mail box for fast packet return CANbus->write(out,CANbus->getLastTxBox()); } else { uint8_t prio = (uint8_t) ((id >> 26) & 0x7); // For high priority messages use first mail box and rest for others. if(prio<4) { ret=CANbus->write(out,CANbus->getFirstTxBox()); else /* jiauka question:

    If prio >=4 it will use any mailbox available, in theory it can fill all available mailboxes
    if there are many packets with prio>=4 rendering the fastpacket stuff uselles or I'm missing
    something ¿?¿?¿?

*/
    ret=CANbus->write(out); // This will use 
return ret;

`

ttlappalainen commented 6 years ago

There are three buffers, high, low and fastpacket. And important is that I set: CANbus->setMailBoxTxBufferSize(CANbus->getFirstTxBox(),HighPriorityBufferSize); CANbus->setMailBoxTxBufferSize(CANbus->getLastTxBox(),FastPacketBufferSize); CANbus->setTxBufferSize(CANGlobalBufSize); So the first two sets buffer size for specific MB. Then there will be global buffer for rest mailboxes. Now on CANbus->write(out), it polls through all MB:s and tests usesGlobalTxRing(index). So for general write it accepts only MB:s, which does not have own buffer. Since I have set mailbox count 3, there will be one for high one for low and one for fast packet. If I would set count to 4, there will be 2 for low priority use. So it will not mesh up anything.

Note that on InitCANFrameBuffers I set FastPacketBufferSize about 9/10 of given size. This is since single frame messages will normally be sent quickly and if you send e.g. temperature data, you do it 1/sec. So you can send burst of e.g. 4 temperature, 2 battery and you are still sending only 6 messages at one time, so you would need 6 frame buffer for low priority. And the first will go immediately to MB, so actually only 5 would be enough. But if you sent one fast packet, you may fill 40 frames very easily. So that needs biggest buffer. And note also that library internal communication may automatically send data. If someone e.g. request "Product information", library responces for it automatically (on ParseMessages) and fills buffers.

My division for buffers is just some guess according my simple tests, but seem to work.

The interrupt could probably work a bit different way. If low priority MB gets empty and high is empty, it could use high priority MB. On the other hand, then high priority MB would not be free, if next command would send high priority message. So that would violate system a bit.

When you get into STM CAN controller, the code is not that difficult that it first looks.

jiauka commented 6 years ago

Thanks!

Noted, and yes STM32 CAN lib is not that hard, the main problem is the zillion families.

ttlappalainen commented 6 years ago

But does they actually have much differencies?

Anyway try to avoid any defines, which affect to library compilation. The problem is that Arduino IDE does not allow you to define compiler defines. So the user has to then modify the library and on next update his defines will disappear. Works better on e.g. PlatformIO, but due to Arduino I still avoid any. That is why I use dynamic buffers etc.

jiauka commented 6 years ago

I have split the library into 2 and mod the HAL driver. https://github.com/jiauka/NMEA2000_stm32f1 and lower level https://github.com/jiauka/STM32F1_CAN

Please, have a look if you have time.

It works fine, at least on a B&G network.

ttlappalainen commented 6 years ago

With a quick look found some questions:

  1. You have buffer sizes as definition. If you compile this in Arduino IDE, they will allways get size 16, since compiler compiles module as own unit.

  2. Do you have any interrupt locking on read/write method? It is possible that just while you are writing to the buffer new frame, interrupt happens and takes a frame from buffer, which then can be meshed up.

  3. I could not find code for setMailBoxTxBufferSize. Is that used?

  4. I normally declare types inside class, if they belong to the class. Then there is no risk that someother uses same name. So you can have: class STM32F1_CAN { puplic: struct CanMsgTypeDef { //... };

jiauka commented 6 years ago

Thanks a lot for your comments!!

Fixed.

If you ever come to Barcelona, let me know and we can sail a litle bit.

ttlappalainen commented 6 years ago

Some more questions.

This means that when it is sending fastpacket, on second byte HAL_CAN_Transmit fails and it returns false to library. This causes library to buffer frame. Since library has only one linear buffer, then single frame messages after fastpacket will be buffered to library buffer and so not send prioritized at all. So instead above you should do:

if ( wait_sent ) { if(HAL_CAN_Transmit(&CanHandle, 10) != HAL_OK) { if(STM32F1_CAN::getInstance().addToRingBuffer(STM32F1_CAN::getInstance().txRingX, msg)==false) { DEBUG(Serial3.println("TX full")); ret= false;; // no more room } } }


Then also on interrupt handlers you have to empty right buffer. And as I mentioned, fastpacket buffer needs to be biggest and there is that fancy formula, which calculates right sizes. Also you initializeBuffers() should create all necessary buffers.

So as far as I see you do not actually use three internal buffers.

2. what does mean 10 on HAL_CAN_Transmit(&CanHandle, 10)?

3. have you checked from chip document the mailbox priority order? In FlexCan MB priorities are so that the last has lowet priority. That is why I have fastpacket MB a last MB on FlexCan. 

MB priority means that if all MB:s are filled with frame waiting for sending data, the highest wins on bus arbitration. So in principle if you have three boxes and fill them all the time, third will not get send at all - I think. This is because when all are full, 1. will be sent. When that gets empty, interrupt will be caused, but 2. will start to send, since it has been handled on hw. So interrupt fills 1. and when 2. has been sent, interrupt will be generated and 1. starts to send on hw. So next interrupt fills 2. etc. So 3. never gets sent. This is just my theory - there may be some other option, which I have not notised.

4. How many tx MB the chip has?
jiauka commented 6 years ago

Hi!

Thanks a lot.

  1. I use 2 internal buffers for prio <=4 and prio >4 RingbufferTypeDef txRing1; RingbufferTypeDef txRing2;

I will add a buffer for the fastpacket stuff too.

  1. The 10 is the timeout in ms, to avoid blocking. 3 & 4 Chip has 3 TX MBOXs, I need to double check priority, but AFAIK, MBOX0 is the lowest one.

yours,

j.

ttlappalainen commented 6 years ago

Sorry I read it wrong - there was ifndef, not ifdef. But blocking is bad, since that really stops other tasks for that time. So best to have third buffer and same handling as with others.

Since you making new "libraries" you could do naming fix. The parameter name wait_sent is totally wrong and it comes from original mcp_can, where I really need to wait until it was sent. That was before I realized that I could reserve one buffer for fastpacket. So you could rename it to something better. E.g. OrderFrames - that would indicate better to "driver", that this frame has to be sent in specific order. Then it is driver decision how to do that.

jiauka commented 6 years ago

Ok, I though it was blocking due to the name, much better to handle it tru a 3rd buffer, will do.

matthijs490 commented 5 years ago

Hi!

I would like to use the STM32F1_CAN library in combination with the NMEA2000 library. So I tried to use the library by Jiauka: https://github.com/jiauka/STM32F1_CAN

After adding the two code snippets to 'NMEA2000_CAN.h' the actisense listener example doesn't build for the STM32F103CB (maple mini).

Am I doing something wrong or are the two libraries no longer compatible?

ttlappalainen commented 5 years ago

There should not be backward incompatible changes. But I can not say, since I have newer tested stm32. It may be also that it does not regognise processor right on NMEA2000_CAN. Note that you do not need to use that, if you do not make code, which should compile with different platforms. Instead you can force code to use can library you need. So instead of including NMEA2000_CAN.h just include and create NMEA2000 object on your own code:

#include <NMEA2000_stm32f1.h>
tNMEA2000 &NMEA2000=*(new NMEA2000_stm32f1());

If that does not compile, then there is something elese wrong.