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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions tests/drivers/candev/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,9 @@ USEMODULE += shell
USEMODULE += can
USEMODULE += isrpipe

# The number of the CAN bus to use can be controlled via environment
# variable CAN_DEV:
CAN_DEV ?= 0
CFLAGS += -DCONFIG_CAN_DEV=$(CAN_DEV)

include $(RIOTBASE)/Makefile.include
89 changes: 53 additions & 36 deletions tests/drivers/candev/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,42 +19,38 @@
* @}
*/

#include <assert.h>
#include <errno.h>
#include <isrpipe.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "shell.h"
#include "test_utils/expect.h"
#include "can/device.h"

#if IS_USED(MODULE_PERIPH_CAN)

#include "periph/can.h"
#include "can_params.h"
#include "board.h"
#include "periph_conf.h"
#include "periph/gpio.h"

static can_t periph_dev;

#elif defined(MODULE_MCP2515)
#include "can_params.h"
#include "candev_mcp2515.h"
#include "mcp2515_params.h"
#include "periph/can.h"
#include "periph_conf.h"
#include "shell.h"
#include "test_utils/expect.h"

static candev_mcp2515_t mcp2515_dev;
/* The params header is only in the include path when the module is used */
#if MODULE_MCP2515
# include "mcp2515_params.h"
#endif

#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 😩

#endif

#define ENABLE_DEBUG 0
#include <debug.h>
#include "debug.h"

/* Default is not using loopback test mode */
#ifndef CONFIG_USE_LOOPBACK_MODE
#define CONFIG_USE_LOOPBACK_MODE 0
# define CONFIG_USE_LOOPBACK_MODE 0
#endif

#ifndef CONFIG_CAN_DEV
# define CONFIG_CAN_DEV 0
#endif

#define RX_RINGBUFFER_SIZE 128 /* Needs to be a power of 2! */
Expand All @@ -63,6 +59,20 @@

static candev_t *candev = NULL;

/* Only one of them is actually used, depending on the driver selected.
* We rely on the compiler to garbage collect the unused */
static can_t periph_dev;
static candev_mcp2515_t mcp2515_dev;

/* The params header is only in the include path when the module is used,
* so we fall back to a NULL ptr if not */
#if MODULE_MCP2515
static candev_mcp2515_conf_t *mcp2515_conf = &candev_mcp2515_conf[CONFIG_CAN_DEV];
#else
static candev_mcp2515_conf_t *mcp2515_conf = NULL;
#endif


Check warning on line 75 in tests/drivers/candev/main.c

View workflow job for this annotation

GitHub Actions / static-tests

too many consecutive empty lines
static int _send(int argc, char **argv)
{
int ret = 0;
Expand Down Expand Up @@ -275,26 +285,33 @@

int main(void)
{

puts("candev test application\n");

isrpipe_init(&rxbuf, (uint8_t *)rx_ringbuf, sizeof(rx_ringbuf));
#if IS_USED(MODULE_PERIPH_CAN)
puts("Initializing CAN periph device");
can_init(&periph_dev, &(candev_conf[0]));
candev = &(periph_dev.candev);
if (IS_USED(MODULE_PERIPH_CAN)) {
puts("Initializing CAN periph device");
can_init(&periph_dev, &(candev_conf[CONFIG_CAN_DEV]));
candev = &(periph_dev.candev);
#if defined(BOARD_SAME54_XPRO)
gpio_init(AT6561_STBY_PIN, GPIO_OUT);
gpio_clear(AT6561_STBY_PIN);
#endif
#elif defined(MODULE_MCP2515)
puts("Initializing MCP2515");
candev_mcp2515_init(&mcp2515_dev, &candev_mcp2515_conf[0]);
candev = (candev_t *)&mcp2515_dev;

#else
/* add initialization for other candev drivers here */
gpio_init(AT6561_STBY_PIN, GPIO_OUT);
gpio_clear(AT6561_STBY_PIN);
#endif
}
else if (IS_USED(MODULE_MCP2515)) {
puts("Initializing MCP2515");
candev_mcp2515_init(&mcp2515_dev, mcp2515_conf);
candev = &mcp2515_dev.candev;
}
else {
/* No CAN driver is used or used CAN driver is not integrated in this
* test yet. We use an undefined function name to let this fail at
* compile time. The conditions above are all compile time constants
* and the compiler will eliminate the dead branches. So if any of them
* matched, this function call will not be part of the compiled object
* file and linking will work. */
extern void the_can_test_apps_depends_on_a_supported_can_driver_but_none_is_used(void);
the_can_test_apps_depends_on_a_supported_can_driver_but_none_is_used();
}

expect(candev);

Expand Down