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 add CAN bus protocol to GRBL source code so that it communicate with multiple devices? #179

Open
mr-huang-mou opened this issue May 21, 2024 · 31 comments

Comments

@mr-huang-mou
Copy link

Will joining the CAN bus protocol have any impact on the GRBL source code?

@terjeio
Copy link
Contributor

terjeio commented May 22, 2024

It should have an impact in that a top level CAN bus API/HAL should be declared/added to the core making it easy to write plugin code that can work across multiple drivers.
@dresco has started working on using CAN with grblHAL but I do not know the state of this work.

@mr-huang-mou
Copy link
Author

It should have an impact in that a top level CAN bus API/HAL should be declared/added to the core making it easy to write plugin code that can work across multiple drivers. @dresco has started working on using CAN with grblHAL but I do not know the state of this work.

I want to burn grblHAL on one development board, which can receive/send data to the CAN bus, and the other two boards are responsible for sending/receiving data from the CAN bus.
My ultimate goal is to realize the operation of other mechanisms before the three-axis mechanism moves, and after other mechanisms have realized their own functions, send instructions to the CAN bus to notify the grblHAL development board that it can perform the movement task.
My other development boards use STM32CubeMX to directly configure the CAN communication protocol. How can I port this configuration directly to grblHAL?
Thank you for your help !!!

@dresco
Copy link

dresco commented Jun 17, 2024

I had an initial go at a CAN bus plugin, and the underlying driver support for STM32F4 and H7.

I will rebase & publish my CAN driver branches for review, and see what Terje thinks of the approach..

@mr-huang-mou
Copy link
Author

mr-huang-mou commented Jun 17, 2024

I had an initial go at a CAN bus plugin, and the underlying driver support for STM32F4 and H7.

I will rebase & publish my CAN driver branches for review, and see what Terje thinks of the approach..

How should I use it for CAN bus communication?Is there any instruction manual I can use?
Thank you for your help!!!

@dresco
Copy link

dresco commented Jun 17, 2024

Is there any instruction manual I can use?

Not yet I'm afraid. There is a very basic my_plugin example that simply printfs received message id's, and sends a test message on each realtime report. I plan to add some more usage examples to the plugin moving forwards.

@terjeio fyi - I've rebased & pushed can driver branches to my F4 & H7 repos..

@mr-huang-mou
Copy link
Author

Is there any instruction manual I can use?

Not yet I'm afraid. There is a very basic my_plugin example that simply printfs received message id's, and sends a test message on each realtime report. I plan to add some more usage examples to the plugin moving forwards.

@terjeio fyi - I've rebased & pushed can driver branches to my F4 & H7 repos..

In addition to adding canbus.c, canbus.h, canbus_ids.h files, what other files need to be added? When I compile, an error is reported and the can.h file is required.

@dresco
Copy link

dresco commented Jun 20, 2024

When I compile, an error is reported and the can.h file is required.

Yes, you'll need the underlying CAN driver support for your platform. My repo with the F4 code is linked in previous comment, however please note it's not been reviewed yet, and may all change before/if it's accepted upstream.

@mr-huang-mou
Copy link
Author

When I compile, an error is reported and the can.h file is required.

Yes, you'll need the underlying CAN driver support for your platform. My repo with the F4 code is linked in previous comment, however please note it's not been reviewed yet, and may all change before/if it's accepted upstream.

How long does it take to use it, or when can you review it?

@terjeio
Copy link
Contributor

terjeio commented Jun 21, 2024

@terjeio fyi - I've rebased & pushed can driver branches to my F4 & H7 repos..

I'll look into this when I am up to speed again - need to clear my backlog first...

@mr-huang-mou
Copy link
Author

mr-huang-mou commented Jun 24, 2024

When I compile, an error is reported and the can.h file is required.

Yes, you'll need the underlying CAN driver support for your platform. My repo with the F4 code is linked in previous comment, however please note it's not been reviewed yet, and may all change before/if it's accepted upstream.

Could you give me the underlying CAN driver support ? I am using the STM32F4 platform. I am very need it.Thank you for your help!!!

@mr-huang-mou
Copy link
Author

mr-huang-mou commented Jun 25, 2024

@terjeio fyi - I've rebased & pushed can driver branches to my F4 & H7 repos..

I'll look into this when I am up to speed again - need to clear my backlog first...

Although I am a student, I can sponsor you a cup of coffee as a reward and thanks for your help.Can you give me your sponsorship information?

@terjeio
Copy link
Contributor

terjeio commented Jun 27, 2024

Can you give me your sponsorship information?

Sorry, I do not have any...

The error about can.h is not found is due to you having the wrong version of the source? It should be in the Inc folder, and can.c in the Src folder. If not there clone this repo (the can branch) instead of the official grblHAL repo.

I have started looking into adding can support, likely moving the plugin part of the code into the core. And I think the plugin should be made a bit more flexible, e.g with filtering and callback support thus removing the need for polling.

@terjeio terjeio reopened this Jun 27, 2024
@mr-huang-mou
Copy link
Author

Can you give me your sponsorship information?

Sorry, I do not have any...

The error about can.h is not found is due to you having the wrong version of the source? It should be in the Inc folder, and can.c in the Src folder. If not there clone this repo (the can branch) instead of the official grblHAL repo.

I have started looking into adding can support, likely moving the plugin part of the code into the core. And I think the plugin should be made a bit more flexible, e.g with filtering and callback support thus removing the need for polling.

Thank you for your help.
How can I use your my_plugin.c?It lacks functions about canbus_queue_tx and canbus_queue_rx @dresco.

@mr-huang-mou

This comment was marked as off-topic.

@terjeio
Copy link
Contributor

terjeio commented Jul 3, 2024

Here is my take on how a CAN API can be added to the core, tested with the F7xx driver. It uses weak functions for the low level driver functions and filters associated with client callbacks for simpler(?) code. To get all messages to one callback use a single filter with both id and mask set to 0.
There is still a some of work to do, I want to add support for extended ids and the low level filtering needs to be sorted out - the current code is just for testing with one or two standard id filters - more will likely fail. For MCUs with no CAN peripheral or a simple one filtering may have to be implemented in the top level code in the core.

can.zip

BTW here is a link to STM32 code with advanced filtering implemented that might be useful.

@dresco
Copy link

dresco commented Jul 3, 2024

Here is my take on how a CAN API can be added to the core...

Thanks, am travelling for a couple of days this week, but will check it out.

As an aside, I notice you've (correctly) commented out the printf() statements I left scattered about for debugging. Is there (or could there be) a standard debug output macro that I should be using? I see other projects using DBG_LOG() or similar that get #defined to whatever output backend is most appropriate.

@mr-huang-mou
Copy link
Author

Here is my take on how a CAN API can be added to the core, tested with the F7xx driver. It uses weak functions for the low level driver functions and filters associated with client callbacks for simpler(?) code. To get all messages to one callback use a single filter with both id and mask set to 0. There is still a some of work to do, I want to add support for extended ids and the low level filtering needs to be sorted out - the current code is just for testing with one or two standard id filters - more will likely fail. For MCUs with no CAN peripheral or a simple one filtering may have to be implemented in the top level code in the core.

can.zip

BTW here is a link to STM32 code with advanced filtering implemented that might be useful.

can.zip has a lot error,when I put it into my project.
something seems to be missing.
image

@terjeio
Copy link
Contributor

terjeio commented Jul 4, 2024

Here is a new version - added some missing includes and improved filter registration.

can 2.zip

I forgot to mention that the GPIO port and pins has to be declared in the board map, an example:

#define CAN_PORT                GPIOD
#define CAN_RX_PIN              0
#define CAN_TX_PIN              1

@mr-huang-mou canbus.dequeue_rx has been replaced with a callback associated with a filter, see my_plugin.c in the Src folder for an example. Some of the errors could be due to the core code you have beeing out of sync with the lates version, if so update from the grblHAL repo.

@mr-huang-mou
Copy link
Author

Here is a new version - added some missing includes and improved filter registration.

can 2.zip

I forgot to mention that the GPIO port and pins has to be declared in the board map, an example:

#define CAN_PORT                GPIOD
#define CAN_RX_PIN              0
#define CAN_TX_PIN              1

@mr-huang-mou canbus.dequeue_rx has been replaced with a callback associated with a filter, see my_plugin.c in the Src folder for an example. Some of the errors could be due to the core code you have beeing out of sync with the lates version, if so update from the grblHAL repo.

Maybe you need to modify the baud rate parameters, because the APB1 clock is 42MHz

@terjeio
Copy link
Contributor

terjeio commented Jul 4, 2024

Is there (or could there be) a standard debug output macro that I should be using?

Yes and no. If there is a free serial stream it can be used, enable in config.h., this makes two output functions available. I guess I should wrap a sprintf() call in an additional one. If no stream is available I usually use report_message() as this outputs the message in a format that should not upset any sender.

BTW I am unsure why CAN_QUEUE_RX_IN_IRQ is an option in your low level code, can_get() gets called from the interrupt handler regardless. Perhaps this option should be the default since when not enabled the CAN interrupt is disabled until the message is read by the foreground process - which may lead to loss of data?

Maybe you need to modify the baud rate parameters, because the APB1 clock is 42MHz

Which driver/board? I am currently testing with a F412 (SuperLongBoard - 25MHz) and a F756 (Nucleo144 - 54MHz), additional clock setups has to be added for other boards that has different APB1 clock frequencies. This may take some time if I am to do it myself, if you can add (or modify) the needed code yourself that would be nice.

@dresco
Copy link

dresco commented Jul 4, 2024

Is there (or could there be) a standard debug output macro that I should be using?

Yes and no. If there is a free serial stream it can be used, enable in config.h., this makes two output functions available. I guess I should wrap a sprintf() call in an additional one. If no stream is available I usually use report_message() as this outputs the message in a format that should not upset any sender.

Yeah I've just been using printf() to output any debug info over SWO when running under a debugger, I guess it would be nice to have a standard macro that could wrap whatever output option is wanted? A disadvantage of my current approach is that there is always the printf() formatting overhead, even if it's not sent anywhere. Was thinking that a macro could be #defined to nothing unless there is actually a chosen output backend?

BTW I am unsure why CAN_QUEUE_RX_IN_IRQ is an option in your low level code, can_get() gets called from the interrupt handler regardless.

I don't think that was the case in my original code (unless I made a mistake - quite possible). Was supposed to be that can_get() was either called in interrupt context from the rx fifo callback, or from the polling loop of the canbus plugin?

Perhaps this option should be the default since when not enabled the CAN interrupt is disabled until the message is read by the foreground process - which may lead to loss of data?

I haven't had a chance to look at the new code carefully yet, but happy to take your lead. My original intent was just to have some flexibility over when received data was moved from the hw fifo buffer into the drivers ringbuffer, was perhaps unnecessary.

Maybe you need to modify the baud rate parameters, because the APB1 clock is 42MHz

Which driver/board? I am currently testing with a F412 (SuperLongBoard - 25MHz) and a F756 (Nucleo144 - 54MHz), additional clock setups has to be added for other boards that has different APB1 clock frequencies. This may take some time if I am to do it myself, if you can add (or modify) the needed code yourself that would be nice.

My initial 45MHz for the F4 driver code came from testing on a F446 Nucleo. Will confess I didn't check APB1 frequencies on other supported boards. Happy to work out some more timings for the F4 variants...

@mr-huang-mou
Copy link
Author

mr-huang-mou commented Jul 5, 2024

Which driver/board? I am currently testing with a F412 (SuperLongBoard - 25MHz) and a F756 (Nucleo144 - 54MHz), additional clock setups has to be added for other boards that has different APB1 clock frequencies. This may take some time if I am to do it myself, if you can add (or modify) the needed code yourself that would be nice.

I am currently testing with a F407(42MHz).I modified the prescaler and TimeSeg1 to correct the baud rate.At first I didn't know why my board couldn't communication with my other boards,so I checked the board I was using,but the configuration was still F407,so I looked at the APB1 and found that my other boards were different from your APB1.I just modified a little bit based on your code,and the subsequent complete version was still your support.Thanks for your help!!!

@mr-huang-mou
Copy link
Author

BTW I am unsure why CAN_QUEUE_RX_IN_IRQ is an option in your low level code, can_get() gets called from the interrupt handler regardless. Perhaps this option should be the default since when not enabled the CAN interrupt is disabled until the message is read by the foreground process - which may lead to loss of data?

Another thing is how to receive data. In my test, I can't know whether my board receives the data sent by other boards. I use an oscilloscope to detect that there is data on the receiving pin PA11, but the receiving interrupt function does not process the data.

@terjeio
Copy link
Contributor

terjeio commented Jul 5, 2024

Was thinking that a macro could be #defined to nothing unless there is actually a chosen output backend?

Something like this? Cannot remember now if it ever worked.

BTW I am unsure why CAN_QUEUE_RX_IN_IRQ is an option in your low level code, can_get() gets called from the interrupt handler regardless.

I don't think that was the case in my original code (unless I made a mistake - quite possible). Was supposed to be that can_get() was either called in interrupt context from the rx fifo callback, or from the polling loop of the canbus plugin?

My bad, you are correct. But since inserting messages in the receive buffer can be safely done from an interrupt context (as long as there is only one source) then I see no reason for continuously polling for them (at a high frequency). My default is to instead post a task to the foreground process when a message is received, which reduces the polling overhead. But IMO better just to insert the message directly into the buffer and leave the interrupt on.

Another thing is how to receive data.

Have you tried with setting the filter mask to 0?
Other boards are running your code? grblHAL as well? On exactly the same boards?
If not the baud rate (bit timing) could be off.
Wiring is correct (polarity)?
I am not sure if this is important but I think so.
Do you have tranceivers in your setup?
Again, I am not sure if this is important. I have not tried with cross connecting RX/TX directly.

@dresco
Copy link

dresco commented Jul 6, 2024

Something like this? Cannot remember now if it ever worked.

Yes, that works here for getting the LwIP output over SWO, or even simpler approach;

#ifdef DEBUG
// driver specific
#define DBG_LOG(...) printf(__VA_ARGS__)
#endif
.
.
.
#ifndef DBG_LOG
#define DBG_LOG(...)
#endif

Though I appreciate can all get a bit more complicated ;)

But IMO better just to insert the message directly into the buffer and leave the interrupt on.

Yep, agreed.

Another thing is how to receive data.

I've done some basic testing on an F4 Nucleo board now, and is working here (once timings defined correctly).

I'll get the H7 driver (FDCAN peripheral) ported across to the same structure, and then update the control panel plugin to use the new format.

@terjeio
Copy link
Contributor

terjeio commented Jul 8, 2024

I have already committed the canbus code, to follow in the next commit is printf style debug output, either to the DEBUGOUT stream (raw) or to the active stream (wrapped to avoid upsetting senders: [MSG: Debug: <msg>]). debug_printf() is the base function, debug_print() a macro that outputs messages if DEBUG or DEBUGOUT is defined. The macro is defined in stream.h.

@dresco
Copy link

dresco commented Jul 9, 2024

Hi @terjeio Just updating the H7 support & noticed a couple of things in the core code?

In canbus_queue_rx(), I think it should be inserting at current head, not next_head, as per canbus_queue_tx()

@@ -96,8 +96,8 @@ ISR_CODE static bool ISR_FUNC(canbus_queue_rx)(canbus_message_t message, can_rx_
     uint8_t next_head = (rx_buffer.head + 1) % CANBUS_BUFFER_LEN;
 
     if((ok = next_head != rx_buffer.tail)) {
-        rx_buffer.rx[next_head].callback = callback;
-        rx_buffer.rx[next_head].message = message;
+        rx_buffer.rx[rx_buffer.head].callback = callback;
+        rx_buffer.rx[rx_buffer.head].message = message;
 
         rx_buffer.head = next_head;
     }

And in canbus_set_baud(), it calls can_set_baud() twice.. Btw, I note that doesn't actually stop/start the CAN peripheral to use the new baud rate any more. I guess that's because all the filters/callbacks would need to be added again?

@@ -132,8 +132,6 @@ static status_code_t canbus_set_baud (setting_id_t id, uint_fast16_t value)
 {
     settings.canbus_baud = value;
 
-    can_set_baud(baud[settings.canbus_baud]);
-
     return can_set_baud(baud[settings.canbus_baud]) ? Status_OK : Status_SettingValueOutOfRange;
 }

@terjeio
Copy link
Contributor

terjeio commented Jul 9, 2024

Just updating the H7 support & noticed a couple of things in the core code?

Oops - getting sloppy with my coding... Fix comitted.

Btw, I note that doesn't actually stop/start the CAN peripheral to use the new baud rate any more. I guess that's because all the filters/callbacks would need to be added again?

Correct, filters/callbacks will be lost - have to find a way to just change the baud rate. For now I just output a message.

@dresco
Copy link

dresco commented Jul 10, 2024

For info, have pushed the updated CAN support for the H7 driver.

Will confess I was struggling to follow the fmi/bank filter logic for the bxCAN peripheral, seems easier for FDCAN (unless I'm missing something, but appears to work as expected).

Thanks for adding the debug output, am now wondering how it can be used for something other than serial streams (i.e. SWO or Segger RTT backends), will open a separate conversation to save getting too off topic here.

@mr-huang-mou
Copy link
Author

mr-huang-mou commented Jul 12, 2024

Just updating the H7 support & noticed a couple of things in the core code?

Oops - getting sloppy with my coding... Fix comitted.

image
I don't know why this function stop excuting after a while(3~6 seconds). I inserted the serial output so that I can observe it.
Since I updated your code that I couldn't excute CAN communication. I know because of baud rate was still 125000,but other device is 500000.I found a problem that baud rate couldn't revise and I received message that value out of range.

@terjeio
Copy link
Contributor

terjeio commented Jul 15, 2024

I don't know why this function stop excuting after a while(3~6 seconds).

Without access to the rest of your code it is impossible to tell.

I found a problem that baud rate couldn't revise and I received message that value out of range.

The baud rate is set with $399 using an id for the different rates:

$$=399
    0 - 125000
    1 - 250000
    2 - 500000
    3 - 1000000

To set it to 500000 you have to use $399=2.

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

3 participants