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

tests/drivers/candev: minor cleanup #21175

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maribu
Copy link
Member

@maribu maribu commented Jan 30, 2025

Contribution description

  • Do not hard code the number of the CAN interface to use. (This also allows specifying it via make command line / environment variable.)
  • Make less use of preprocessor and rely on compiler to eliminate dead branches and unused variables.

Testing procedure

The test should behave as before. (Unless one would pass e.g. CAN_DEV=1, in which case the 2nd CAN interface would be used instead of the first.)

Issues/PRs references

None

@maribu maribu added Area: tests Area: tests and testing framework Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jan 30, 2025
@riot-ci
Copy link

riot-ci commented Jan 30, 2025

Murdock results

FAILED

8d0ac30 tests/drivers/candev: minor cleanup

Success Failures Total Runtime
5 1 18 18s
Build failures (1)
Application Target Toolchain Runtime (s) Worker
tests/drivers/candev atmega256rfr2-xpro gnu 0.77 alien

Artifacts

- Do not hard code the number of the CAN interface to use. (This
  also allows specifying it via make command line / environment
  variable.)
- Make less use of preprocessor and rely on compiler to eliminate dead
  branches and unused variables.
@maribu maribu force-pushed the tests/drivers/candev branch from 99715d7 to 8d0ac30 Compare January 30, 2025 10:06
#else
/* add includes for other candev drivers here */
#ifdef BOARD_SAME54_XPRO
# include "periph/gpio.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why only for this board?

Copy link
Member Author

Choose a reason for hiding this comment

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

That is annoying, isn't it?

I'll add a __attribute__((weak)) board_enable_can_transceiver(can_t *dev); and a __attribute__((weak)) board_disable_can_transceiver(can_t *dev); into the periph_can driver, so that we do not need to have magic stuff at application level, but rather let the board hook into can_init().

But that is not independent of this PR and will be a follow up.

Copy link
Contributor

Choose a reason for hiding this comment

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

We already have a CAN transceiver interface, it's just not integrated with candev 😩

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants