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

drivers: can: socketCAN: move code from drivers to its own file #41847

Conversation

alexanderwachter
Copy link
Member

Cleanup the drivers from socketCAN and make its own driver that instantiates the net interface.

samples/net/sockets/can/Kconfig Outdated Show resolved Hide resolved
subsys/net/lib/sockets/sockets_can.c Outdated Show resolved Hide resolved
subsys/net/lib/sockets/sockets_can.c Outdated Show resolved Hide resolved
subsys/net/lib/sockets/sockets_can.c Outdated Show resolved Hide resolved
@alexanderwachter alexanderwachter force-pushed the can/socketCAN/removeCodeFromDrivers branch from 7c68a2c to 4498ad6 Compare January 16, 2022 15:35
@alexanderwachter
Copy link
Member Author

Doc CI fail is unrelated.

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.

Nice clean-up, just a few minor observations.

drivers/can/can_socketcan.c Outdated Show resolved Hide resolved
drivers/can/can_socketcan.c Outdated Show resolved Hide resolved
drivers/can/can_socketcan.c Outdated Show resolved Hide resolved
samples/net/sockets/can/Kconfig Outdated Show resolved Hide resolved
@henrikbrixandersen henrikbrixandersen added this to the v3.0.0 milestone Jan 16, 2022
@henrikbrixandersen henrikbrixandersen added the Release Notes To be mentioned in the release notes label Jan 16, 2022
@alexanderwachter alexanderwachter force-pushed the can/socketCAN/removeCodeFromDrivers branch from 4498ad6 to 0c0d163 Compare January 16, 2022 16:47
drivers/can/Kconfig Outdated Show resolved Hide resolved
drivers/can/can_socketcan.c Outdated Show resolved Hide resolved
@henrikbrixandersen henrikbrixandersen linked an issue Jan 28, 2022 that may be closed by this pull request
@SebastianBoe SebastianBoe removed their request for review January 31, 2022 11:14
@tejlmand tejlmand removed their request for review February 3, 2022 11:31
@henrikbrixandersen henrikbrixandersen removed this from the v3.0.0 milestone Mar 8, 2022
@henrikbrixandersen
Copy link
Member

Tx issues. API sample works fine.

@alexanderwachter Any progress on this? Any more information on the errors, you are seeing? Are they also present on main?

@henrikbrixandersen
Copy link
Member

Tx issues. API sample works fine.

@alexanderwachter Any progress on this? Any more information on the errors, you are seeing? Are they also present on main?

@alexanderwachter Please provide further details on the issues you are seeing?

@alexanderwachter alexanderwachter force-pushed the can/socketCAN/removeCodeFromDrivers branch from 051a0aa to 38888b7 Compare April 6, 2022 20:44
@alexanderwachter alexanderwachter removed the DNM This PR should not be merged (Do Not Merge) label Apr 6, 2022
@alexanderwachter
Copy link
Member Author

@henrikbrixandersen it was a stack overflow, caused by log immediate and a -115. I used the wrong symbol for the loopback Kconfig (forgot the CONFIG_ prefix).
However, works now!

@alexanderwachter alexanderwachter force-pushed the can/socketCAN/removeCodeFromDrivers branch from 38888b7 to d2be9e8 Compare April 6, 2022 20:52
jukkar
jukkar previously approved these changes Apr 7, 2022
drivers/can/CMakeLists.txt Outdated Show resolved Hide resolved
drivers/can/Kconfig Outdated Show resolved Hide resolved
drivers/can/can_socketcan.c Outdated Show resolved Hide resolved
drivers/can/can_socketcan.c Outdated Show resolved Hide resolved
samples/net/sockets/can/Kconfig Outdated Show resolved Hide resolved
@@ -176,6 +180,10 @@ static int setup_socket(void)
return -ENOENT;
}

#ifdef CONFIG_SAMPLE_SOCKET_CAN_LOOPBACK_MODE
can_set_mode(DEVICE_DT_GET(DT_CHOSEN(zephyr_canbus)), CAN_LOOPBACK_MODE);
Copy link
Member

Choose a reason for hiding this comment

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

With loopback mode, we should be able to remove the harness from samples/net/sockets/can/sample.yaml and add a console fixture for validating the sample in CI.

Move the code for socket instanciation from each driver
to a generic driver, that makes an instance of a socketCAN
net device for the chosen node.

Signed-off-by: Alexander Wachter <alexander@wachter.cloud>
Add loopback mode config to the sample so that the code
can be tested without an actual CAN network, but wit a
single board.

Signed-off-by: Alexander Wachter <alexander@wachter.cloud>
Do not log immediate in the sample, because it causes stack overflows.

Signed-off-by: Alexander Wachter <alexander@wachter.cloud>
@henrikbrixandersen henrikbrixandersen requested review from dleach02, jukkar and karstenkoenig and removed request for dleach02 April 7, 2022 16:35
@carlescufi carlescufi merged commit b008916 into zephyrproject-rtos:main Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SocketCAN not supported for NUCLEO H743ZI
6 participants