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

v1 STM32 bxCAN driver. #12

Merged
merged 24 commits into from Sep 29, 2020
Merged

Conversation

TomDeRybel
Copy link
Contributor

Hello Pavel,

As discussed, here is the pull request for the bxCAN driver.

Best regards,
Tom

Copy link
Member

@pavel-kirienko pavel-kirienko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the liberty to push to your fork directly moving the driver into a subdirectory bxcan (because we're expecting an FDCAN driver eventually to arrive) and updating the README a little. I did not touch the code, as you can see in the commit history.

After this is merged, I am planning to add a basic CI pipeline that would run the clang tools.

stm32/libcanard/bxcan/src/bxcan.c Outdated Show resolved Hide resolved
stm32/libcanard/bxcan/src/bxcan.c Outdated Show resolved Hide resolved
stm32/libcanard/bxcan/src/bxcan.c Outdated Show resolved Hide resolved
stm32/libcanard/bxcan/src/bxcan.c Outdated Show resolved Hide resolved
stm32/libcanard/bxcan/src/bxcan.h Outdated Show resolved Hide resolved
stm32/libcanard/bxcan/src/bxcan.c Outdated Show resolved Hide resolved
stm32/libcanard/bxcan/src/bxcan.c Outdated Show resolved Hide resolved
stm32/libcanard/bxcan/src/bxcan.c Outdated Show resolved Hide resolved
stm32/libcanard/bxcan/src/bxcan.c Outdated Show resolved Hide resolved
stm32/libcanard/bxcan/src/bxcan.c Outdated Show resolved Hide resolved
TomDeRybel and others added 15 commits September 23, 2020 10:22
Co-authored-by: Pavel Kirienko <pavel.kirienko@gmail.com>
Co-authored-by: Pavel Kirienko <pavel.kirienko@gmail.com>
Co-authored-by: Pavel Kirienko <pavel.kirienko@gmail.com>
Co-authored-by: Pavel Kirienko <pavel.kirienko@gmail.com>
Co-authored-by: Pavel Kirienko <pavel.kirienko@gmail.com>
Co-authored-by: Pavel Kirienko <pavel.kirienko@gmail.com>
Co-authored-by: Pavel Kirienko <pavel.kirienko@gmail.com>
Co-authored-by: Pavel Kirienko <pavel.kirienko@gmail.com>
Co-authored-by: Pavel Kirienko <pavel.kirienko@gmail.com>
Co-authored-by: Pavel Kirienko <pavel.kirienko@gmail.com>
@TomDeRybel
Copy link
Contributor Author

Hello Pavel,

I've made the changes. Please have a go at the comments, there are one or two things there.
This is my first time using GitHub and git, so I hope I didn't mess it up.

stm32/libcanard/bxcan/src/bxcan.c Outdated Show resolved Hide resolved
stm32/libcanard/bxcan/src/bxcan.c Show resolved Hide resolved
stm32/libcanard/bxcan/src/bxcan.c Outdated Show resolved Hide resolved
stm32/libcanard/bxcan/src/bxcan.c Outdated Show resolved Hide resolved
@TomDeRybel
Copy link
Contributor Author

TomDeRybel commented Sep 24, 2020

Hello Pavel,
Once more, here are all the changes as I understand them.

It continues to amaze me how something so broken (previous versions) manages to work well enough that none of the problems showed-up yet. Probable due to the very light bus load and low transmission interval I'm running at at this time (just the heartbeat messages of two or three devices on an otherwise clean and short bus).

Copy link
Member

@pavel-kirienko pavel-kirienko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please run clang-format once more? There are some minor formatting issues.

stm32/libcanard/bxcan/src/bxcan.c Outdated Show resolved Hide resolved
Copy link
Member

@pavel-kirienko pavel-kirienko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from one remaining issue with the timeouts, the driver looks okay, but as you said correctly there is too much logic to validate by just looking at it. Would you be interested in collaborating on #13 after this PR is merged? The plan is that we would set up test hardware connected to some computer and make it run various tests automatically on the hardware. It's no rocket science but takes some dedication to arrange the many needed pieces correctly.

stm32/libcanard/bxcan/src/bxcan.c Outdated Show resolved Hide resolved
@TomDeRybel
Copy link
Contributor Author

Regarding #13, I've given it some thought over the weekend. Where I may be willing to contribute is in (custom) test hardware for the project (Such as AVR, STM32, etc... (FD)CAN-enabled boards with the required diagnostics connections, a bus disturbance generation device, etc...) However, explicitly not in writing code. In any case, I'm willing to discuss a role in this.

@pavel-kirienko pavel-kirienko merged commit 95cb7d5 into OpenCyphal-Garage:master Sep 29, 2020
This was referenced Sep 29, 2020
@pavel-kirienko pavel-kirienko changed the title Initial commit of v1 STM32 bxCAN driver. v1 STM32 bxCAN driver. Sep 29, 2020
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

Successfully merging this pull request may close these issues.

None yet

2 participants