Will get to this after #1068 is merged.

One problem with this PR is that it tries to patch upstream library - that apparently would complicate maintenance and upgrade. Was that patch tried to be pushed upstream instead?

Also, the branch appears to be not rebased properly, so over time it accumulated unrelated changes which makes merging complicated.

2015-04-05 14:43 GMT+02:00 Paul Sokolovsky notifications@github.com:

One problem with this PR is that it tries to patch upstream library - that
apparently would complicate maintenance and upgrade. Was that patch tried
to be pushed upstream instead?

Yes it can be an issue, but providing a patch upstream is not easy either.
The code comes from the STM32CUBE library and there does not exist a open
project to send patches to. If I should send a patch to ST-Micro I should
probably test it with all the rest of the ST code in the file. But
micropython does not use any of the CAN reception logic in the HAL code
since it does not utilize the full functionality of the HW.

Also, the branch appears to be not rebased properly, so over time it
accumulated unrelated changes which makes merging complicated.

If/when the code can be accepted I will fix any ageing issues with it.

—
Reply to this email directly or view it on GitHub
#1079 (comment)
.

One problem with this PR is that it tries to patch upstream library - that apparently would complicate maintenance and upgrade. Was that patch tried to be pushed upstream instead?

Although there is precedent for this (changing the hal), as we have made other patches to the hal library in the past. i.e. we fixed some of the arithmetic in the sd code to work properly witth 64-bit, made some changes around SysTick, and fixed some SPI stuff.

So I would still definitely try to avoid making changes to the hal, if its possible to do so, but some things can't be avoided.

In this case I have added on line in one function. One way to avoid this change is to implement the same functionality (in a better way) in the mpy stmhal code. It is a pretty simple function, but in my eyes not very well done. But it may cause portability issues.

I have done an alternative implementation of this functionality by moving the send functionality from the hal to stmhal/can.c. It can be viewed here.
HenrikSolver@5c6e552
It also solves a issue with the original hal code: in the case where a call to send times out, the message is still scheduled to be sent. In the new version the transmit is aborted.

Thanks!

Actually, I added my comment to draw @dpgeorge's attention that we have some backlogs of PRs, and after 1.4 release, it would be nice to give them some attention ;-). Especially that only @dpgeorge can decide whether to patch vendor libs, and how much.

Thanks @PappaPeppar. I merged your more recent version with the CAN_Transmit function in can.c; see 7d5e342.