include: can: Always provide z_impl_can_recover()

[sebhub]

Where the CAN API tests were configured with

CONFIG_CAN_MANUAL_RECOVERY_MODE = n

the tests did not link due to:

classic.c:1351: undefined reference to `z_impl_can_recover'
classic.c:1128: undefined reference to `z_impl_can_recover'
classic.c:1142: undefined reference to `z_impl_can_recover'

Fix this by always providing a z_impl_can_recover() implementation.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[henrikbrixandersen]

Always having this defined will shift what is now a build time error to a runtime error, which is not desired.

Why do you change the configuration of the test?

[sebhub]

Always having this defined will shift what is now a build time error to a runtime error, which is not desired.

In tests/drivers/can/api/src/classic.c we have:

/**
 * @brief Test CAN controller bus recovery.
 *
 * It is not possible to provoke a bus off state, but verify the API call return codes.
 */
ZTEST_USER(can_classic, test_recover)
{
	can_mode_t cap;
	int err;

	Z_TEST_SKIP_IFNDEF(CONFIG_CAN_MANUAL_RECOVERY_MODE);

	err = can_get_capabilities(can_dev, &cap);
	zassert_equal(err, 0, "failed to get CAN capabilities (err %d)", err);

	if ((cap & CAN_MODE_MANUAL_RECOVERY) != 0U) {
		/* Check that manual recovery fails when not in manual recovery mode */
		err = can_recover(can_dev, TEST_RECOVER_TIMEOUT);
		zassert_equal(err, -ENOTSUP, "wrong error return code (err %d)", err);

This Z_TEST_SKIP_IFNDEF(CONFIG_CAN_MANUAL_RECOVERY_MODE); doesn't work in this case. What would also work is making this test conditional with an #ifdef CONFIG_CAN_MANUAL_RECOVERY_MODE.

Why do you change the configuration of the test?

I would like to run the tests with exactly the configuration which is later used by the application. Also I am not sure how the recovery handler should be implemented for the new MPFS CAN driver.

[henrikbrixandersen]

This Z_TEST_SKIP_IFNDEF(CONFIG_CAN_MANUAL_RECOVERY_MODE); doesn't work in this case. What would also work is making this test conditional with an #ifdef CONFIG_CAN_MANUAL_RECOVERY_MODE.

Ah. That's leftover in the existing test suites, but I can see how that make give you the impression that you can disable CONFIG_CAN_MANUAL_RECOVERY_MODE and still expect the test suite to compile. I have fixed that in #98098

[henrikbrixandersen]

I would like to run the tests with exactly the configuration which is later used by the application.

This is not the purpose of the API test suite.

Also I am not sure how the recovery handler should be implemented for the new MPFS CAN driver.

That's not a valid reason for breaking this for everyone else, though. If the MPFS CAN controller does not support manual bus off recovery, don't implement it. It's optional as long as you do not include CAN_MODE_MANUAL_RECOVERY in the capabilities reported from the driver via can_get_capabilities().

[sebhub]

I would like to run the tests with exactly the configuration which is later used by the application.

This is not the purpose of the API test suite.

Also I am not sure how the recovery handler should be implemented for the new MPFS CAN driver.

That's not a valid reason for breaking this for everyone else, though. If the MPFS CAN controller does not support manual bus off recovery, don't implement it. It's optional as long as you do not include CAN_MODE_MANUAL_RECOVERY in the capabilities reported from the driver via can_get_capabilities().

I would not call the change from a link-time to a run-time error a break for everyone else. The test code suggested to me that it should work.

It would be even clearer to remove the can_recover() declaration to also get compile-time errors/warnings if someone calls can_recover() where CONFIG_CAN_MANUAL_RECOVERY_MODE is not enabled.

[sebhub]

This Z_TEST_SKIP_IFNDEF(CONFIG_CAN_MANUAL_RECOVERY_MODE); doesn't work in this case. What would also work is making this test conditional with an #ifdef CONFIG_CAN_MANUAL_RECOVERY_MODE.

Ah. That's leftover in the existing test suites, but I can see how that make give you the impression that you can disable CONFIG_CAN_MANUAL_RECOVERY_MODE and still expect the test suite to compile. I have fixed that in #98098

Thanks, this clarifies the code.

Would a change like this be acceptable:

commit fbabc1e4ac6b34c7b19dbedacc669eb0b0d8832a (HEAD -> can-recover)
Author: Sebastian Huber <[email protected]>
Date:   Wed Oct 22 04:14:48 2025 +0200

    tests: drivers: can: api: Optional CAN recover
    
    Where the CAN API tests are configured with
    
    CONFIG_CAN_MANUAL_RECOVERY_MODE = n
    
    the tests cannot use can_recover() since no implementation is provided.
    Make the tests related to can_recover() conditional.
    
    Signed-off-by: Sebastian Huber <[email protected]>

diff --git a/tests/drivers/can/api/src/classic.c b/tests/drivers/can/api/src/classic.c
index e934fa34517..5c8057af78c 100644
--- a/tests/drivers/can/api/src/classic.c
+++ b/tests/drivers/can/api/src/classic.c
@@ -1110,6 +1110,7 @@ ZTEST_USER(can_classic, test_send_fd_format)
        zassert_equal(err, -ENOTSUP, "sent a CAN FD format frame in non-FD mode");
 }
 
+#ifdef CONFIG_CAN_MANUAL_RECOVERY_MODE
 /**
  * @brief Test CAN controller bus recovery.
  *
@@ -1162,6 +1163,7 @@ ZTEST_USER(can_classic, test_recover)
                zassert_equal(err, -ENOSYS, "wrong error return code (err %d)", err);
        }
 }
+#endif /* CONFIG_CAN_MANUAL_RECOVERY_MODE */
 
 /**
  * @brief Test retrieving the state of the CAN controller.
@@ -1330,6 +1332,7 @@ ZTEST_USER(can_classic, test_start_while_started)
        zassert_equal(err, -EALREADY, "wrong error return code (err %d)", err);
 }
 
+#ifndef CONFIG_CAN_MANUAL_RECOVERY_MODE
 /**
  * @brief Test recover is not allowed while started.
  */
@@ -1357,6 +1360,7 @@ ZTEST_USER(can_classic, test_recover_while_stopped)
        err = can_start(can_dev);
        zassert_equal(err, 0, "failed to start CAN controller (err %d)", err);
 }
+#endif /* CONFIG_CAN_MANUAL_RECOVERY_MODE */
 
 /**
  * @brief Test sending is not allowed while stopped.

[henrikbrixandersen]

Would a change like this be acceptable:

No, it would not. The test suite depends on CONFIG_CAN_MANUAL_RECOVERY_MODE=y since it tests the behaviour of the sys call even if the driver does not support manual bus recovery.

[sebhub]

Would a change like this be acceptable:

No, it would not. The test suite depends on CONFIG_CAN_MANUAL_RECOVERY_MODE=y since it tests the behaviour of the sys call even if the driver does not support manual bus recovery.

With this change, you would be able to run the test suite also with CONFIG_CAN_MANUAL_RECOVERY_MODE=n at the cost of having four lines for the C preprocessor. Anyway, this patch is easy to maintain externally.

[henrikbrixandersen]

It would be even clearer to remove the can_recover() declaration to also get compile-time errors/warnings if someone calls can_recover() where CONFIG_CAN_MANUAL_RECOVERY_MODE is not enabled.

For future reference, that would be against our API design guidelines: https://docs.zephyrproject.org/latest/develop/api/design_guidelines.html#conditional-data-and-apis: "Function declarations that are available only when the option is enabled should be provided unconditionally."