From 2836ba6acb82c1f3730fc253fceafca365229ec7 Mon Sep 17 00:00:00 2001 From: Tiago Lourinho Date: Wed, 2 Sep 2026 10:31:41 +0200 Subject: [PATCH 1/3] added constructor to can device configuration --- src/include/CanDeviceConfiguration.h | 13 +++++++++++++ test/cpp/CanDevice_test.cpp | 16 ++++++++++------ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/include/CanDeviceConfiguration.h b/src/include/CanDeviceConfiguration.h index be194608..1fdc43e5 100644 --- a/src/include/CanDeviceConfiguration.h +++ b/src/include/CanDeviceConfiguration.h @@ -16,6 +16,19 @@ * depending on the type of CAN device being used. */ struct CanDeviceConfiguration { + CanDeviceConfiguration() = default; + + /** + * @brief Constructs a configuration from a map of string parameters, where + * each key must be the name of one of the parameters below + * + * @param parameters The configuration parameters, indexed by name. + * @throws std::invalid_argument if a key is not a configuration parameter or + * if a value cannot be converted to the type of its parameter. + */ + explicit CanDeviceConfiguration( + const std::map& parameters); + /** * @brief Builds a configuration from a map of string parameters, where * each key must be the name of one of the parameters below. diff --git a/test/cpp/CanDevice_test.cpp b/test/cpp/CanDevice_test.cpp index c94512aa..5f4cc0db 100644 --- a/test/cpp/CanDevice_test.cpp +++ b/test/cpp/CanDevice_test.cpp @@ -40,7 +40,8 @@ TEST_F(CanDeviceTest, CreationLoopbackDevice) { auto dummy_cb_ = [](const CanFrame& frame) { return; }; auto myDevice = CanDevice::create( "loopback", - CanDeviceArguments{CanDeviceConfiguration{"dummy"}, dummy_cb_}); + CanDeviceArguments{CanDeviceConfiguration{{{"bus_name", "dummy"}}}, + dummy_cb_}); ASSERT_NE(myDevice, nullptr); ASSERT_EQ(myDevice->vendor_name(), "loopback"); ASSERT_EQ(myDevice->args().config.bus_name.value(), "dummy"); @@ -56,7 +57,8 @@ TEST_F(CanDeviceTest, LoopbackDeviceMessageTransmission) { }; auto myDevice = CanDevice::create( "loopback", - CanDeviceArguments{CanDeviceConfiguration{"dummy"}, dummy_cb_}); + CanDeviceArguments{CanDeviceConfiguration{{{"bus_name", "dummy"}}}, + dummy_cb_}); for (uint32_t i = 0; i < 10; ++i) { outFrames.push_back(CanFrame{i}); @@ -86,8 +88,9 @@ TEST_F(CanDeviceTest, OnErrorCallbackIsInvoked) { }; TestableCanDevice device{ - "test", CanDeviceArguments{CanDeviceConfiguration{"dummy"}, - [](const CanFrame&) {}, on_error_cb}}; + "test", + CanDeviceArguments{CanDeviceConfiguration{{{"bus_name", "dummy"}}}, + [](const CanFrame&) {}, on_error_cb}}; device.notify_error(CanReturnCode::disconnected); ASSERT_TRUE(called); @@ -108,8 +111,9 @@ TEST_F(CanDeviceTest, ThrowingCallbacksAreHandled) { }; TestableCanDevice device{ - "test", CanDeviceArguments{CanDeviceConfiguration{"dummy"}, receiver_cb, - on_error_cb}}; + "test", + CanDeviceArguments{CanDeviceConfiguration{{{"bus_name", "dummy"}}}, + receiver_cb, on_error_cb}}; ASSERT_NO_THROW(device.received(CanFrame{0})); ASSERT_TRUE(receiver_called); From 589861fac3aebb9bf27f09514a93d9cbb4cea1e6 Mon Sep 17 00:00:00 2001 From: Tiago Lourinho Date: Wed, 2 Sep 2026 14:32:45 +0200 Subject: [PATCH 2/3] added warning about ignored parameters --- src/include/CanDevice.h | 14 ++++++++++ src/include/CanDeviceConfiguration.h | 8 ++++++ src/include/CanVendorAnagate.h | 7 +++++ src/include/CanVendorSocketCan.h | 7 +++++ src/include/CanVendorSocketCanSystec.h | 7 +++++ src/main/CanDevice.cpp | 19 ++++++++++++++ src/main/CanDeviceConfiguration.cpp | 36 ++++++++++++++++++++++++++ src/main/CanVendorAnagate.cpp | 7 +++++ src/main/CanVendorSocketCan.cpp | 5 ++++ src/main/CanVendorSocketCanSystec.cpp | 6 +++++ 10 files changed, 116 insertions(+) diff --git a/src/include/CanDevice.h b/src/include/CanDevice.h index fe429e22..a2257f5a 100644 --- a/src/include/CanDevice.h +++ b/src/include/CanDevice.h @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -72,6 +73,19 @@ struct CanDevice { std::string_view vendor, const CanDeviceArguments& configuration); protected: + /** + * @brief Logs a warning for every configuration parameter that was + * provided but is not in the vendor's list of accepted parameters. + * + * @param vendor The name of the vendor the parameters are checked against. + * @param config The configuration provided by the user. + * @param accepted_parameters The names of the parameters the vendor takes + * into account. + */ + static void warn_ignored_parameters( + std::string_view vendor, const CanDeviceConfiguration& config, + const std::set& accepted_parameters) noexcept; + /** * @brief Constructor for the CanDevice class. * diff --git a/src/include/CanDeviceConfiguration.h b/src/include/CanDeviceConfiguration.h index 1fdc43e5..007cccec 100644 --- a/src/include/CanDeviceConfiguration.h +++ b/src/include/CanDeviceConfiguration.h @@ -6,6 +6,8 @@ #include #include #include +#include +#include /** * @brief Configuration structure for a CanDevice. @@ -122,6 +124,12 @@ struct CanDeviceConfiguration { std::optional sent_acknowledgement; std::string to_string() const noexcept; + + /** + * @brief The name and value of every parameter that was provided. + */ + std::vector> set_parameters() + const noexcept; }; std::ostream& operator<<(std::ostream& os, diff --git a/src/include/CanVendorAnagate.h b/src/include/CanVendorAnagate.h index b1f9a674..251f9cbe 100644 --- a/src/include/CanVendorAnagate.h +++ b/src/include/CanVendorAnagate.h @@ -5,6 +5,8 @@ #include #include #include //NOLINT +#include +#include #include #include "AnaGateDllCan.h" @@ -24,6 +26,11 @@ struct CanVendorAnagate : CanDevice { AnaInt32 nBufferLen, AnaInt32 nFlags, AnaInt32 hHandle) noexcept; + /** + * @brief The configuration parameters this vendor takes into account. + */ + static const std::set accepted_parameters; + explicit CanVendorAnagate(const CanDeviceArguments& configuration); inline ~CanVendorAnagate() override { vendor_close(); } diff --git a/src/include/CanVendorSocketCan.h b/src/include/CanVendorSocketCan.h index 591a0f82..dae346b0 100644 --- a/src/include/CanVendorSocketCan.h +++ b/src/include/CanVendorSocketCan.h @@ -7,6 +7,8 @@ #include #include +#include +#include #include // NOLINT #include "CanDevice.h" @@ -22,6 +24,11 @@ * methods to open, close, and send CAN frames using the SocketCAN interface. */ struct CanVendorSocketCan : CanDevice { + /** + * @brief The configuration parameters this vendor takes into account. + */ + static const std::set accepted_parameters; + explicit CanVendorSocketCan(const CanDeviceArguments& args); ~CanVendorSocketCan() { vendor_close(); } diff --git a/src/include/CanVendorSocketCanSystec.h b/src/include/CanVendorSocketCanSystec.h index 396fd94a..b7bef603 100644 --- a/src/include/CanVendorSocketCanSystec.h +++ b/src/include/CanVendorSocketCanSystec.h @@ -2,6 +2,8 @@ #define SRC_INCLUDE_CANVENDORSOCKETCANSYSTEC_H_ #include +#include +#include #include "CanDevice.h" #include "CanVendorSocketCan.h" @@ -18,6 +20,11 @@ * SocketCan due to a kernel-panic bug on Systec linux module. */ struct CanVendorSocketCanSystec : CanDevice { + /** + * @brief The configuration parameters this vendor takes into account. + */ + static const std::set accepted_parameters; + explicit CanVendorSocketCanSystec(const CanDeviceArguments& args); ~CanVendorSocketCanSystec() { vendor_close(); } diff --git a/src/main/CanDevice.cpp b/src/main/CanDevice.cpp index 204b70ea..3884d29d 100644 --- a/src/main/CanDevice.cpp +++ b/src/main/CanDevice.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -15,6 +16,18 @@ #include "CanVendorSocketCanSystec.h" #endif +void CanDevice::warn_ignored_parameters( + std::string_view vendor, const CanDeviceConfiguration& config, + const std::set& accepted_parameters) noexcept { + for (const auto& [name, value] : config.set_parameters()) { + if (accepted_parameters.count(name) == 0) { + LOG(Log::WRN, CanLogIt::h()) + << "Ignoring configuration parameter " << name << "=" << value + << ": it is not accepted by vendor " << vendor; + } + } +} + /** * @brief Opens the CAN device for communication. * @@ -156,17 +169,23 @@ std::unique_ptr CanDevice::create( #ifndef _WIN32 if (vendor == "socketcan") { LOG(Log::DBG, CanLogIt::h()) << "Creating SocketCAN CAN device"; + warn_ignored_parameters(vendor, configuration.config, + CanVendorSocketCan::accepted_parameters); return std::make_unique(configuration); } if (vendor == "socketcan_systec") { LOG(Log::DBG, CanLogIt::h()) << "Creating SocketCAN Systec CAN device"; + warn_ignored_parameters(vendor, configuration.config, + CanVendorSocketCanSystec::accepted_parameters); return std::make_unique(configuration); } #endif if (vendor == "anagate") { LOG(Log::DBG, CanLogIt::h()) << "Creating Anagate CAN device"; + warn_ignored_parameters(vendor, configuration.config, + CanVendorAnagate::accepted_parameters); return std::make_unique(configuration); } diff --git a/src/main/CanDeviceConfiguration.cpp b/src/main/CanDeviceConfiguration.cpp index 5244e1bd..b2bcb6f8 100644 --- a/src/main/CanDeviceConfiguration.cpp +++ b/src/main/CanDeviceConfiguration.cpp @@ -9,6 +9,8 @@ #include #include #include +#include +#include namespace { @@ -190,3 +192,37 @@ std::ostream& operator<<(std::ostream& os, const CanDeviceConfiguration& config) noexcept { return os << config.to_string(); } + +/** + * @brief Lists the name and value of every parameter that was provided. + * + * @return A vector of (name, value) pairs, one for each optional field that + * holds a value. + */ +std::vector> +CanDeviceConfiguration::set_parameters() const noexcept { + std::vector> parameters; + + if (bus_name.has_value()) + parameters.emplace_back("bus_name", bus_name.value()); + if (bus_number.has_value()) + parameters.emplace_back("bus_number", std::to_string(bus_number.value())); + if (host.has_value()) parameters.emplace_back("host", host.value()); + if (bitrate.has_value()) + parameters.emplace_back("bitrate", std::to_string(bitrate.value())); + if (enable_termination.has_value()) + parameters.emplace_back("enable_termination", + enable_termination.value() ? "true" : "false"); + if (high_speed.has_value()) + parameters.emplace_back("high_speed", + high_speed.value() ? "true" : "false"); + if (timeout.has_value()) + parameters.emplace_back("timeout", std::to_string(timeout.value())); + if (vcan.has_value()) + parameters.emplace_back("vcan", vcan.value() ? "true" : "false"); + if (sent_acknowledgement.has_value()) + parameters.emplace_back("sent_acknowledgement", + std::to_string(sent_acknowledgement.value())); + + return parameters; +} diff --git a/src/main/CanVendorAnagate.cpp b/src/main/CanVendorAnagate.cpp index 40af348d..c58c5ea1 100644 --- a/src/main/CanVendorAnagate.cpp +++ b/src/main/CanVendorAnagate.cpp @@ -6,6 +6,7 @@ #include #include #include // NOLINT +#include #include #include #include @@ -15,6 +16,12 @@ std::mutex CanVendorAnagate::m_handles_lock; std::map CanVendorAnagate::m_handles; +const std::set CanVendorAnagate::accepted_parameters = { + "bus_number", "host", + "bitrate", "enable_termination", + "high_speed", "sent_acknowledgement", + "timeout"}; + /** * @brief Callback function to handle incoming CAN frames from the AnaGate DLL. * diff --git a/src/main/CanVendorSocketCan.cpp b/src/main/CanVendorSocketCan.cpp index 1078c4ee..d6cf9dea 100644 --- a/src/main/CanVendorSocketCan.cpp +++ b/src/main/CanVendorSocketCan.cpp @@ -11,6 +11,8 @@ #include #include +#include +#include #include // NOLINT #include @@ -21,6 +23,9 @@ constexpr auto EPOLL_WAIT_CYCLE_MS = 1000; constexpr auto LIBSOCKETCAN_ERROR = -1; constexpr auto LIBSOCKETCAN_SUCCESS = 0; +const std::set CanVendorSocketCan::accepted_parameters = { + "bus_name", "bitrate", "vcan", "timeout"}; + /** * @brief Constructor for the CanVendorSocketCan class. * diff --git a/src/main/CanVendorSocketCanSystec.cpp b/src/main/CanVendorSocketCanSystec.cpp index dc9d7818..b8ff0604 100644 --- a/src/main/CanVendorSocketCanSystec.cpp +++ b/src/main/CanVendorSocketCanSystec.cpp @@ -4,6 +4,12 @@ #include #include +#include +#include + +const std::set CanVendorSocketCanSystec::accepted_parameters = + CanVendorSocketCan::accepted_parameters; + /** * @brief Constructor for the CanVendorSocketCanSystec class. * From 5d944199fce67c971f4c71334a2c7dbc1d4b48ec Mon Sep 17 00:00:00 2001 From: Tiago Lourinho Date: Fri, 4 Sep 2026 09:42:00 +0200 Subject: [PATCH 3/3] Revert "added constructor to can device configuration" This reverts commit 2836ba6acb82c1f3730fc253fceafca365229ec7. --- src/include/CanDeviceConfiguration.h | 13 ------------- test/cpp/CanDevice_test.cpp | 16 ++++++---------- 2 files changed, 6 insertions(+), 23 deletions(-) diff --git a/src/include/CanDeviceConfiguration.h b/src/include/CanDeviceConfiguration.h index 007cccec..12bc3b24 100644 --- a/src/include/CanDeviceConfiguration.h +++ b/src/include/CanDeviceConfiguration.h @@ -18,19 +18,6 @@ * depending on the type of CAN device being used. */ struct CanDeviceConfiguration { - CanDeviceConfiguration() = default; - - /** - * @brief Constructs a configuration from a map of string parameters, where - * each key must be the name of one of the parameters below - * - * @param parameters The configuration parameters, indexed by name. - * @throws std::invalid_argument if a key is not a configuration parameter or - * if a value cannot be converted to the type of its parameter. - */ - explicit CanDeviceConfiguration( - const std::map& parameters); - /** * @brief Builds a configuration from a map of string parameters, where * each key must be the name of one of the parameters below. diff --git a/test/cpp/CanDevice_test.cpp b/test/cpp/CanDevice_test.cpp index 5f4cc0db..c94512aa 100644 --- a/test/cpp/CanDevice_test.cpp +++ b/test/cpp/CanDevice_test.cpp @@ -40,8 +40,7 @@ TEST_F(CanDeviceTest, CreationLoopbackDevice) { auto dummy_cb_ = [](const CanFrame& frame) { return; }; auto myDevice = CanDevice::create( "loopback", - CanDeviceArguments{CanDeviceConfiguration{{{"bus_name", "dummy"}}}, - dummy_cb_}); + CanDeviceArguments{CanDeviceConfiguration{"dummy"}, dummy_cb_}); ASSERT_NE(myDevice, nullptr); ASSERT_EQ(myDevice->vendor_name(), "loopback"); ASSERT_EQ(myDevice->args().config.bus_name.value(), "dummy"); @@ -57,8 +56,7 @@ TEST_F(CanDeviceTest, LoopbackDeviceMessageTransmission) { }; auto myDevice = CanDevice::create( "loopback", - CanDeviceArguments{CanDeviceConfiguration{{{"bus_name", "dummy"}}}, - dummy_cb_}); + CanDeviceArguments{CanDeviceConfiguration{"dummy"}, dummy_cb_}); for (uint32_t i = 0; i < 10; ++i) { outFrames.push_back(CanFrame{i}); @@ -88,9 +86,8 @@ TEST_F(CanDeviceTest, OnErrorCallbackIsInvoked) { }; TestableCanDevice device{ - "test", - CanDeviceArguments{CanDeviceConfiguration{{{"bus_name", "dummy"}}}, - [](const CanFrame&) {}, on_error_cb}}; + "test", CanDeviceArguments{CanDeviceConfiguration{"dummy"}, + [](const CanFrame&) {}, on_error_cb}}; device.notify_error(CanReturnCode::disconnected); ASSERT_TRUE(called); @@ -111,9 +108,8 @@ TEST_F(CanDeviceTest, ThrowingCallbacksAreHandled) { }; TestableCanDevice device{ - "test", - CanDeviceArguments{CanDeviceConfiguration{{{"bus_name", "dummy"}}}, - receiver_cb, on_error_cb}}; + "test", CanDeviceArguments{CanDeviceConfiguration{"dummy"}, receiver_cb, + on_error_cb}}; ASSERT_NO_THROW(device.received(CanFrame{0})); ASSERT_TRUE(receiver_called);