Fix for #3863: STM Check can sync error #4329

adustm · 2017-05-16T11:34:45Z

Description

Can init function needs 11 bits at the same level to synchronize to the CAN bus.
In some cases this cannot occur (refer to #3863 where the object is initialized at a lower frequency than the bus and the bus is very busy). We thus need a timeout mechanism to exit the init function.
in TARGET_STM/can_api.c:

        /* Get tick */
        tickstart = HAL_GetTick();
        can->BTR = btr;
        can->MCR &= ~(uint32_t)CAN_MCR_INRQ;
        while ((can->MSR & CAN_MSR_INAK) == CAN_MSR_INAK) {
            if((HAL_GetTick() - tickstart ) > 2)
            {
                status = 0;
                break;
            }
        }
        if (status ==0) {
            error("can ESR  0x%04x.%04x + timeout status %d", (can->ESR&0XFFFF0000)>>16, (can->ESR&0XFFFF), status);
        }

Status

READY

Migrations

NO

Steps to test or reproduce

refer to #3863 : need a 1st device that overloads the CAN bus with a high frequency. Then try to initialize the 2nd device with a low frequency (or the default one at 100kHz).
The CAN object of the 2nd device will not output of the init function because the HW synchronization cannot occur
With this PR, you will be notified that the initialization could not occur.

cc @bperry730 @ohagendorf

ohagendorf · 2017-05-16T12:12:06Z

I would implement the timeout handling in both while loops, i.e. also after line 200.

adustm · 2017-05-16T14:18:11Z

I would implement the timeout handling in both while loops, i.e. also after line 200.

@ohagendorf I have added that for safety check, but the HW synchronization occurs only when going from init_mode to normal_mode. That's why I had put the timeout only on the 2nd while loop.

Ingramz

I found some code style inconsistencies, it would be nice if you could fix those please.

Ingramz · 2017-05-17T12:35:07Z

targets/TARGET_STM/can_api.c

-    can_frequency(obj, 100000);
+    if (can_frequency(obj, 100000) != 1) {
+        error("Can frequency could not be set\n");
+    };


; is redundant

Ingramz · 2017-05-17T12:36:16Z

targets/TARGET_STM/can_api.c

        while ((can->MSR & CAN_MSR_INAK) != CAN_MSR_INAK) {
+            if((HAL_GetTick() - tickstart ) > 2)


Whitespace missing between if and (, also there's a redundant whitespace after tickstart.

Ingramz · 2017-05-17T12:36:49Z

targets/TARGET_STM/can_api.c

        while ((can->MSR & CAN_MSR_INAK) != CAN_MSR_INAK) {
+            if((HAL_GetTick() - tickstart ) > 2)
+            {


Opening bracket should be on the same line with if

Ingramz · 2017-05-17T12:37:28Z

targets/TARGET_STM/can_api.c

+            /* Get tick */
+            tickstart = HAL_GetTick();
+            while ((can->MSR & CAN_MSR_INAK) == CAN_MSR_INAK) {
+                if((HAL_GetTick() - tickstart ) > 2)


Same issues as above.

Ingramz · 2017-05-17T12:37:44Z

targets/TARGET_STM/can_api.c

+                    break;
+                }
+            }
+            if (status ==0) {


One space is missing.

Ingramz · 2017-05-17T12:38:59Z

targets/TARGET_STM/can_api.c

+                }
+            }
+            if (status ==0) {
+                error("can ESR  0x%04x.%04x + timeout status %d", (can->ESR&0XFFFF0000)>>16, (can->ESR&0XFFFF), status);


Would add spaces around & and >>, also use lowercase 0x prefix instead of 0X.

Ingramz · 2017-05-17T12:39:18Z

targets/TARGET_STM/can_api.c

    } else {
-        return 0;
+        status=0;


Missing spaces.

Ingramz · 2017-05-22T11:22:09Z

Very good! There's still a redundant space on those lines: if ((HAL_GetTick() - tickstart ) > 2) {, change them to if ((HAL_GetTick() - tickstart) > 2) { and it will be perfect.

adbridge · 2017-05-25T11:12:46Z

/morph test

mbed-bot · 2017-05-25T12:42:55Z

Result: SUCCESS

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

/morph test

Output

mbed Build Number: 342

All builds and test passed!

sg- · 2017-05-26T22:09:21Z

@adustm This needs a conflict resolved.

add a timeout + return an error message

adustm · 2017-05-29T08:28:44Z

@sg- Rebase done. Thanks

adustm · 2017-05-30T11:36:58Z

Hello, Is it ok now ?

0xc0170 · 2017-05-30T11:56:08Z

/morph test

mbed-bot · 2017-05-30T13:31:14Z

Result: SUCCESS

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

/morph test

Output

mbed Build Number: 368

All builds and test passed!

adbridge · 2017-06-05T14:59:08Z

Seems this is reliant on #4165 which had an API change and was thus targeted for 5.5.0

0xc0170 added the needs: review label May 16, 2017

Ingramz suggested changes May 17, 2017

View reviewed changes

Ingramz approved these changes May 23, 2017

View reviewed changes

adbridge added needs: CI and removed needs: review labels May 25, 2017

0xc0170 added ready for merge and removed needs: CI labels May 25, 2017

0xc0170 approved these changes May 25, 2017

View reviewed changes

adustm added 6 commits May 29, 2017 10:24

Handle can_frequency sync error

52d38a1

add a timeout + return an error message

Give access to HAL_GetTick function

897625c

Optimize the use of the status value

9a1d055

Add timeout also on the 1st while loop

b99ccf6

Fix coding style

795bfd2

Fix another typo

6770678

adustm force-pushed the can_sync_error branch from ac579b6 to 6770678 Compare May 29, 2017 08:25

0xc0170 added needs: CI and removed ready for merge labels May 30, 2017

0xc0170 added ready for merge and removed needs: CI labels May 30, 2017

adbridge added the release-version: 5.4.7 label May 30, 2017

adbridge removed the ready for merge label May 30, 2017

adbridge merged commit 45b4d41 into ARMmbed:master May 30, 2017

adbridge added release-version: 5.5.0-rc1 and removed release-version: 5.4.7 labels Jun 5, 2017

adustm deleted the can_sync_error branch January 15, 2018 16:18

		while ((can->MSR & CAN_MSR_INAK) != CAN_MSR_INAK) {
		if((HAL_GetTick() - tickstart ) > 2)

Fix for #3863: STM Check can sync error #4329

Fix for #3863: STM Check can sync error #4329

Uh oh!

Conversation

adustm commented May 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Status

Migrations

Steps to test or reproduce

Uh oh!

ohagendorf commented May 16, 2017

Uh oh!

adustm commented May 16, 2017

Uh oh!

Ingramz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ingramz commented May 22, 2017

Uh oh!

adbridge commented May 25, 2017

Uh oh!

mbed-bot commented May 25, 2017

Output

Uh oh!

sg- commented May 26, 2017

Uh oh!

adustm commented May 29, 2017

Uh oh!

adustm commented May 30, 2017

Uh oh!

0xc0170 commented May 30, 2017

Uh oh!

mbed-bot commented May 30, 2017

Output

Uh oh!

adbridge commented Jun 5, 2017

Uh oh!

Uh oh!

adustm commented May 16, 2017 •

edited

Loading