Skip to content

CANBUS driver for native_posix board #14196

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

CANBUS driver for native_posix board #14196

wants to merge 2 commits into from

Conversation

@jukkar
Copy link
Member

@jukkar jukkar commented Mar 8, 2019 •
edited
Loading

Edit: I am now able to receive and send CAN packets in Zephyr. The Linux vcan virtual interface is the peer in this case. My earlier problems were because filters were not set properly. Currently this only supports Socket-CAN interface in Zephyr, we can add support for Zephyr native API later.

In Linux one can use cansend to send CAN packets like this:

./cansend zcan 001#11.2233.44556677.88

To receive CAN packets do this:

./candump zcan,1:1

The candump and cansend are part of the can-utils package https://github.com/linux-can/can-utils
You also need the can-setup.sh script from Zephyr net-tools repo, I will send new version for it shortly.

This PR contains two extra commits (padding and const ones) that I already sent separately, just ignore them.

@jukkar jukkar requested a review from pfalcon as a code owner March 8, 2019 14:27
@jukkar jukkar changed the title [WIP] CANBUS driver for native_posix board CANBUS driver for native_posix board Mar 8, 2019
@codecov-io
Copy link

Codecov Report

Merging #14196 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14196      +/-   ##
==========================================
- Coverage   52.68%   52.67%   -0.01%     
==========================================
  Files         307      307              
  Lines       45473    45473              
  Branches    10530    10530              
==========================================
- Hits        23956    23955       -1     
  Misses      16640    16640              
- Partials     4877     4878       +1
Impacted Files Coverage Δ
include/can.h 100% <100%> (ø) ⬆️
arch/posix/core/posix_core.c 90.9% <0%> (-1.02%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 10bdc2a...1f57d35. Read the comment docs.

@aescolar aescolar added the area: native port Host native arch port (native_sim) label Mar 8, 2019
aescolar
aescolar reviewed Mar 8, 2019
View reviewed changes
Copy link
Member

@aescolar aescolar left a comment

Choose a reason for hiding this comment

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

would you like to add a paragraph or two of documentation?

Otherwise just a couple of very minor comments after a quick lock

drivers/can/Kconfig.native_posix Outdated
Copy link
Member

Choose a reason for hiding this comment

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

minor: name in +the+ host system.

drivers/can/canbus_native_posix_adapt.c Outdated
Copy link
Member

Choose a reason for hiding this comment

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

would a #warning or #error under an #else make sense here?

drivers/can/Kconfig.native_posix Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Sidecomment: If you want you can add a command line option to set it, similarly to:
https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/bluetooth/hci/userchan.c#L239

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the hint. That looks like a way to go.

@jukkar
Copy link
Member Author

jukkar commented Mar 8, 2019

would you like to add a paragraph or two of documentation?

Yes, I will create documentation for this. I just didn't had time to create one today. This is probably not going to 1.14 anyway.

pfalcon
pfalcon approved these changes Mar 13, 2019
View reviewed changes
Copy link
Contributor

@pfalcon pfalcon left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this. Wouldn't have a chance to play with this soon, so just approving.

@jukkar jukkar added this to the v1.15.0 milestone Mar 14, 2019
jukkar added 2 commits April 17, 2019 23:42
@jukkar
drivers: can: Add support for native_posix board
e93bc66 
This initial version supports SocketCAN. The driver passes
CANBUS data between Linux vcan virtual CAN driver and Zephyr.

You also need to use can-setup.sh script from the net-tools
project in order to make communication work with Linux vcan
driver.

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar
samples: net: sockets: can: Add native_posix board support
1814682 
Add native_posix board support to socket-can sample application.

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar jukkar force-pushed the canbus-driver-for-native_posix branch from 1f57d35 to 1814682 Compare April 17, 2019 20:43
@jukkar
Copy link
Member Author

jukkar commented Apr 17, 2019

Rebased against latest master.

@jukkar
Copy link
Member Author

jukkar commented Apr 17, 2019

This is not ready yet as I need to still solve the issues @aescolar mentioned.

@jukkar jukkar added the DNM This PR should not be merged (Do Not Merge) label Apr 17, 2019
@galak galak removed this from the v2.0.0 milestone Aug 19, 2019
@aescolar
Copy link
Member

@jukkar Note that this PR is getting stale

@jukkar
Copy link
Member Author

jukkar commented Sep 17, 2019

It is stale already and would need more TLC. Let's see if I have time to work more with it.

@jukkar
Copy link
Member Author

jukkar commented Oct 24, 2019

Storing this to jukkar#21 in order to cleanup the upstream list of PRs.

@jukkar jukkar closed this Oct 24, 2019
@rxdu rxdu mentioned this pull request Nov 6, 2020
Open
@rdu-weston rdu-weston mentioned this pull request Feb 23, 2021
Merged
@pllee4-weston pllee4-weston mentioned this pull request Jul 21, 2021
Closed
@pllee4-weston
Copy link

hi @jukkar can this issue be reopened? We have rebased your previous work and hope it can go upstream.

@jukkar
Copy link
Member Author

jukkar commented Jul 21, 2021

@pllee4-weston Please create a new pull request with your changes, it is not possible to reopen this PR as the underlying branch has been deleted from github.

@pllee4-weston
Copy link

@jukkar Alright thanks for the information.

@pllee4-weston pllee4-weston mentioned this pull request Jul 22, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aescolar aescolar aescolar left review comments

@tbursztyka tbursztyka Awaiting requested review from tbursztyka

@alexanderwachter alexanderwachter Awaiting requested review from alexanderwachter

+1 more reviewer

@pfalcon pfalcon pfalcon approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area: CAN area: native port Host native arch port (native_sim) area: Networking DNM This PR should not be merged (Do Not Merge)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jukkar @codecov-io @aescolar @pllee4-weston @pfalcon @galak