Skip to content

boards: posix: native_posix: enable can_loopback0 by default #50530

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
Merged
merged 1 commit into from
Oct 3, 2022
Merged

boards: posix: native_posix: enable can_loopback0 by default #50530

merged 1 commit into from
Oct 3, 2022

Conversation

@martinjaeger
Copy link
Member

@martinjaeger martinjaeger commented Sep 22, 2022 •
edited
Loading

The .yaml file states that CAN is supported and zephyr,canbus is chosen in the dts, but building the basic sample application samples/drivers/can/counter fails without additional configuration.

The loopback driver does not require any additional steps like the linux SocketCAN driver, so it is safe to enable it by default and get rid of the many overlay files in the tests.

@martinjaeger martinjaeger added area: CAN bug The issue is a bug, or the PR is fixing a bug labels Sep 22, 2022
@zephyrbot zephyrbot added area: Networking area: Sockets Networking sockets area: native port Host native arch port (native_sim) labels Sep 22, 2022
henrikbrixandersen
henrikbrixandersen previously approved these changes Sep 22, 2022
View reviewed changes
Copy link
Member

@henrikbrixandersen henrikbrixandersen left a comment

Choose a reason for hiding this comment

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

Thanks. I left it as disabled since the loopback driver was not sufficient to run all the test suites back then. Should be fine now, though.

jukkar
jukkar previously approved these changes Sep 22, 2022
View reviewed changes
@martinjaeger
Copy link
Member Author

martinjaeger commented Sep 22, 2022

Thanks. I left it as disabled since the loopback driver was not sufficient to run all the test suites back then. Should be fine now, though.

Ok, I see. Now we get several failing tests which were previously just not run.

The counter sample should be excluded from the tests because it keeps running for ever and thus times out, but the ISO-TP failures may be actual issues with the loopback driver or ISO-TP itself.

I'd suggest to exclude the native_posix board from the ISO-TP tests for now.

@martinjaeger martinjaeger force-pushed the native-posix-can-loopback-default branch from fde03cd to cbab91a Compare September 22, 2022 14:17
@martinjaeger
boards: posix: native_posix: enable can_loopback0 by default
3a280ba 
The .yaml file states that CAN is supported, but the basic sample
application samples/drivers/can/counter cannot be compiled without
additional configuration.

The loopback driver does not require any additional steps like the
linux SocketCAN driver, so it is safe to enable it by default and
get rid of the many overlay files in the tests.

ISO-TP tests and the counter sample are excluded via .yaml from
twister tests because of timing issues.

Signed-off-by: Martin Jäger <[email protected]>
@martinjaeger martinjaeger force-pushed the native-posix-can-loopback-default branch from cbab91a to 3a280ba Compare September 22, 2022 18:18
@martinjaeger martinjaeger mentioned this pull request Sep 23, 2022
Closed
@martinjaeger
Copy link
Member Author

martinjaeger commented Sep 23, 2022

For some reason samples/drivers/can/counter works well if run with west, but fails in Twister, so I've excluded it from the CI for now and opened an additional bug report: #50570

@henrikbrixandersen
Copy link
Member

henrikbrixandersen commented Sep 23, 2022

We really need to look into the ISO-TP protocol implementation post v3.2. The tests are failing on multiple boards. Either the implementation is flawed or the tests are.

@henrikbrixandersen henrikbrixandersen removed the bug The issue is a bug, or the PR is fixing a bug label Sep 23, 2022
@henrikbrixandersen
Copy link
Member

henrikbrixandersen commented Sep 23, 2022 •
edited
Loading

I have removed the "bug" label as I do not see this as a bug. We have filtering in place (filter: dt_chosen_enabled("zephyr,canbus")) to skip CAN tests/samples without an enabled CAN controller.

@fabiobaltieri fabiobaltieri added this to the v3.3.0 milestone Sep 23, 2022
@fabiobaltieri
Copy link
Member

fabiobaltieri commented Sep 23, 2022

Hey, tagged for v3.3.0, feel free to update if you think this should be considered a bugfix.

@carlescufi carlescufi merged commit 9cec058 into zephyrproject-rtos:main Oct 3, 2022
@martinjaeger martinjaeger deleted the native-posix-can-loopback-default branch October 16, 2022 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@henrikbrixandersen henrikbrixandersen henrikbrixandersen approved these changes

@jukkar jukkar jukkar approved these changes

@str4t0m str4t0m str4t0m approved these changes

@alexanderwachter alexanderwachter Awaiting requested review from alexanderwachter

@rlubos rlubos Awaiting requested review from rlubos

@tbursztyka tbursztyka Awaiting requested review from tbursztyka

@pfalcon pfalcon Awaiting requested review from pfalcon

@aescolar aescolar Awaiting requested review from aescolar

@daor-oti daor-oti Awaiting requested review from daor-oti

@karstenkoenig karstenkoenig Awaiting requested review from karstenkoenig

@mengxianglinx mengxianglinx Awaiting requested review from mengxianglinx

@nashif nashif Awaiting requested review from nashif

Assignees

@henrikbrixandersen henrikbrixandersen

Labels

area: CAN area: native port Host native arch port (native_sim) area: Networking area: Sockets Networking sockets

Projects

None yet

Milestone

v3.3.0

Development

Successfully merging this pull request may close these issues.

7 participants

@martinjaeger @henrikbrixandersen @fabiobaltieri @jukkar @str4t0m @carlescufi @zephyrbot