Update of can_api.c fixing #2987

[martinjaeger]

Description

Fix issue #2987

Status

Bug fix is completely implemented for STM32F0 and has been tested on STM32F072 Nucleo board.

Migrations

No APIs are changed, but the behavior of the STM32F0 CAN filter API behaves now as it would be expected.

Todos

• Check Reference Manuals of other MCUs from STM32 family if register layout is the same as F0 MCU. If yes, same issue will exist for other STM32 targets supporting CAN.

Deploy notes

No changes in build environment.

Steps to test or reproduce

Try to set a CAN message filter with the previous version and wonder why it filters random message IDs, just not the ones you want to be filtered.

[0xc0170]

@bcostm @adustm @LMESTM please review

[bcostm]

LGTM
Just some minor typos.
Thanks for this PR.

[martinjaeger]

Should I fix the typos and make a new pull request? Or should I just keep calm and wait for someone to merge it and fix the typos?

[bridadan]

@bcostm Could you be more specifc on the typos so @martinjaeger can fix them? Thanks!

[bcostm]

It's ok you can merge.

[bridadan]

/morph test

[mbed-bot]

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 960

All builds and test passed!

[0xc0170]

Please fix the formatting, look at the other functions, for instance void can_init(can_t *obj, PinName rd, PinName td)

[0xc0170]

@martinjaeger Can you please sign https://developer.mbed.org/contributor_agreement/ ?

[martinjaeger]

Just the mentioned example void can_init(can_t *obj, PinName rd, PinName td) was not consistent regarding formatting of if and while statements. Changed format based on the mbed coding style.

Hope now all typos are fine. If not please explain in a bit more detail what I'm doing wrong. Thanks.

Contributor agreement is signed (mbed user martinj).

[0xc0170]

@mbed-bot: TEST

HOST_OSES=ALL
BUILD_TOOLCHAINS=ALL
TARGETS=ALL

[mbed-bot]

[Build 1074]
FAILURE: Something went wrong when building and testing.

[0xc0170]

@mbed-bot: TEST

HOST_OSES=ALL
BUILD_TOOLCHAINS=ALL
TARGETS=ALL

[mbed-bot]

[Build 1075]
SUCCESS: Building succeeded and tests were run! Be sure to check the test results

[sg-]

@marhil01 @jupe Can you share feedback about the pr-head CI failure?

[marhil01]

Seems that problem is in test infra/framework

[mgiaco]

Hello,

So if this is a bug for STM32F0 i think there is also a problem for the other STM32 targets because the bxCAN is the almost the same on every STMF as I know.

[bridadan]

Hi @mgiaco, Are you using CAN on other ST families? If so, can you confirm that your seeing the original issue?

[mgiaco]

Hello, no I have not used mbed-os till now. But I will made some tests in the next days with an STM32F4. I will report but first I need a solution for debugging on mbed-os. I tried to export a simple test project with GCC_ARM or with the ST Toolchain everything is broken as i found out. I have no keil toolchain so I have to wait for a fix. After that i will try it.

But I report this because I use STM32F4 am STM32F1 bare metal with CAN and the bxCAN is almost the same so the filter mechanism is also the same as you can see in the hal also.

[bridadan]

Sounds good, looking forward to your results!

[mgiaco]

Hello,
Today I made some test on a custom eval board which is based on a NUCLEOF103RB. So now CAN is working but for me there is a issue in PeripheralPins.c. Also the filter is not correct in my opinion.

So the RX pins for CAN should be mode input not mode alternate function push pull. I do not test PA_11 but I think that should be also mode input.

so go with this

const PinMap PinMap_CAN_RD[] = {
    {PA_11, CAN_1, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_NOPULL, 0)},
    {PB_8 , CAN_1, STM_PIN_DATA(STM_MODE_INPUT, GPIO_NOPULL, 10)}, // Remap CAN_RX to PB_8
    {NC,    NC,    0}
};

or this

const PinMap PinMap_CAN_RD[] = {
    {PA_11, CAN_1, STM_PIN_DATA(STM_MODE_INPUT, GPIO_NOPULL, 0)},
    {PB_8 , CAN_1, STM_PIN_DATA(STM_MODE_INPUT, GPIO_NOPULL, 10)}, // Remap CAN_RX to PB_8
    {NC,    NC,    0}
};

But I can test PA11 in the next days also. I will also made some filter tests later.
I can made a pull request in the next days I think. I will also test the F4 if there is a possability to configure mbed for a stm32F4 discovery board. Is there any way to do that?

[bridadan]

@mgiaco thanks for looking into this, and we look forward to your coming pull request!

I believe most stm32F4 boards are already supported by mbed OS, so I don't think you should have any trouble running tests!

[mgiaco]

Yes I know, but till now I use the online compiler - there I add a project. Publish it and after that I am able to clone it with mbed-cli. And now I cannot use STM32F4 discovery so I have to modify something by hand I think - I am right?

By the way is there any other way go get projects from mbed? Is it possible to use git instead of mercurial?
How can I config which target, lib, etc. I am using. I would be really nice if it is possible to clone mbed-os and do anything local.

cheers

[bridadan]

Publish it and after that I am able to clone it with mbed-cli. And now I cannot use STM32F4 discovery so I have to modify something by hand I think - I am right?

If you haven't used any device specific code, you should be able to just recompile the project for the F4 board. Please see the mbed CLI documentation here: https://docs.mbed.com/docs/mbed-os-handbook/en/5.2/getting_started/blinky_cli/

[martinjaeger]

I just checked the reference manuals of STM32F0, STM32F1, STM32F2, STM32F3, STM32F4 and STM32F7. All have the same filter configuration, so the bug should be present for all of them. My pull request fixed only STM32F0.

Apart from that, I realized that returning the handle in the fixed function can cause some problems, as the handle number could be 0. This would be interpreted as "not successful" according to CAN.h mbed OS 5 API documentation. So this should be fixed aswell and return 1 in case of successfully set filter.

(BTW: There seem to be some issues with the doxygen output in above page...)

How do we proceed? Should I fix the function for all STM32 versions and make a new pull request or do we need to test it in hardware with all different Nucleo boards? I have only the STM32F072 available.

[mgiaco]

What handle do you mean exactly?

[0xc0170]

How do we proceed? Should I fix the function for all STM32 versions and make a new pull request or do we need to test it in hardware with all different Nucleo boards? I have only the STM32F072 available.

Definitely create a bug report, and if fixed, it should be tested on various platforms if the fix applies to them. ST team that I tagged above might be able to help.

[bcostm]

Hi,
I think it's time to factorize the code and create a single can_api.c file to cover all STM32 devices.

@martinjaeger did you start already to modify the can file for other stm32 ? If not I can start to do it. And we can help for sure to test on different stm32 boards.

By the way, did you develop a test to check the CAN filter ? If yes can you share it ? Thanks.

[mgiaco]

@bcostm That is a great idea. Last week I tried mbed can on a nucleo f103rb after the changes I commented above it works for me. But as you can read in this issue #3505 there is really some stuf which is missing in this api. So maybe with this refactoring also some api issues can be fixed.

At work I use this general can api https://github.com/canpie/CANpie for an in house canstack. I also made an example for a stm32f4 discovery board https://github.com/liebherrnenzing/stm32f4_canpie-fd which shows the basic usage of the api. Here https://github.com/liebherrnenzing/canpie-fd_device/blob/bded4dd846454f174104c261ef9534782d089782/stm32/f4/hal/stm32f4xx_canpie-fd.c#L929 you can find the filter implementation which works for me. The nice thing about canpie is that you get the information which buffer send or receive the message. That is what I am missing most in the mbed can implementation.

[martinjaeger]

@bcostm no, didn't start coding. A single can_api.c sounds like a great idea.

I had some very simple test code and an additional separate USB2CAN device to inject and read messages to/from the bus.

Code snippet:

#include "mbed.h"

Serial serial(USBTX, USBRX);

DigitalOut led1(LED1);
DigitalOut led2(PA_10);

Ticker tick1;

//CAN can1(PA_11, PA_12);  // RX, TX
CAN can1(PB_8, PB_9);  // RX, TX

int counter = 0;

void send();
void setup();
void receive();

int main()
{
    setup();
    tick1.attach(&send, 0.5);
    can1.attach(&receive);
    can1.mode(CAN::Normal);
    //can1.mode(CAN::LocalTest);  // loop back mode

    can1.filter(0x1, 0xFFFF, CANStandard, 0);

    led1 = 1;

    while(1) {
        serial.printf("loop()\n");
        wait(1);
    }
}

void send() {
    if(can1.write(CANMessage(1337, &counter, 1))) {
        counter++;
        serial.printf("Message sent: %d\n", can1.tderror());
        led1 = !led1;
    }
}

void receive() {
    CANMessage msg;
    if(can1.read(msg)) {
        serial.printf("Message received. id: %d, data: %d\n", msg.id, msg.data[0]);
        led2 = !led2;
    }
}

void setup()
{
    serial.baud(115200);
    serial.printf("\nSerial interface started...\n");
}