-
Notifications
You must be signed in to change notification settings - Fork 3k
Fix #3463 CAN read() return value #3492
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
Conversation
@Nodraak Can you please sign https://developer.mbed.org/contributor_agreement/ and share a nickname on mbed? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change definitely looks good to me
@Nodraak the change sounds more than reasonable - thanks for posting it ! |
@0xc0170 so I guess I've accepted the contributor agreement https://developer.mbed.org/users/Nodraak/ |
/morph test |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
// check FPM0 which holds the pending message count in FIFO 0 | ||
// if no message is pending, return 0 | ||
if ((can->RF0R & CAN_RF0R_FMP0) == 0) | ||
return 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small code style change, it should be surrounded by {
and }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I squash my commit or make a new one ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preferably squashed (edit the commit that changes this lines).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I won't have my computer until the 2nd of January, ping me the 3rd if I forget
/morph test |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
99ed8d1
to
885b018
Compare
Just squashed the code style change + rebased master (~70 new commits since) |
/morph test
Thanks |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
Ports for Upcoming Targets Fixes and Changes 3488: Dev stm i2c v2 unitary functions ARMmbed/mbed-os#3488 3492: Fix #3463 CAN read() return value ARMmbed/mbed-os#3492 3503: [LPC15xx] Ensure that PWM=1 is resolved correctly ARMmbed/mbed-os#3503 3504: [LPC15xx] CAN implementation improvements ARMmbed/mbed-os#3504 3539: NUCLEO_F412ZG - Add support of TRNG peripheral ARMmbed/mbed-os#3539 3540: STM: SPI: Initialize Rx in spi_master_write ARMmbed/mbed-os#3540 3438: K64F: Add support for SERIAL ASYNCH API ARMmbed/mbed-os#3438 3519: MCUXpresso: Fix ENET driver to enable interrupts after interrupt handler is set ARMmbed/mbed-os#3519 3544: STM32L4 deepsleep improvement ARMmbed/mbed-os#3544 3546: NUCLEO-F412ZG - Add CAN peripheral ARMmbed/mbed-os#3546 3551: Fix I2C driver for RZ/A1H ARMmbed/mbed-os#3551 3558: K64F UART Asynch API: Fix synchronization issue ARMmbed/mbed-os#3558 3563: LPC4088 - Fix vector checksum ARMmbed/mbed-os#3563 3567: Dev stm32 F0 v1.7.0 ARMmbed/mbed-os#3567 3577: Fixes linking errors when building with debug profile ARMmbed/mbed-os#3577
Description
This PR includes two commits which fix two things both related to the CAN read() method, but if you prefer two separate PRs, I can do it without any problem.
1 - CAN read() return value
The first commit fixes bug #3463 (CAN read() return value).
Disclaimer: Although this PR as only been tested on nucleo F303K8 and L476RG, I expect others series to work too.
2 - bit set vs. register reset
The second commit prevent the reset of the whole
RF0R
register. I am not sure what is the impact, since at first my PR seemed to be working without, but I think that at least it does no harm.