From 22fa1994986a9731a272844b81586c18bdf54959 Mon Sep 17 00:00:00 2001 From: Tiago Lourinho Date: Mon, 14 Sep 2026 12:41:41 +0200 Subject: [PATCH 1/4] added interface up check during open --- src/main/CanVendorSocketCan.cpp | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/main/CanVendorSocketCan.cpp b/src/main/CanVendorSocketCan.cpp index d6cf9dea..7ad6ebc7 100644 --- a/src/main/CanVendorSocketCan.cpp +++ b/src/main/CanVendorSocketCan.cpp @@ -101,10 +101,29 @@ CanReturnCode CanVendorSocketCan::vendor_open() noexcept { return CanReturnCode::internal_api_error; } + // ifr_ifindex and ifr_flags share the same union in struct ifreq, so save + // the index before it gets overwritten by the SIOCGIFFLAGS call below. + const int ifindex = ifr.ifr_ifindex; + + if (ioctl(m_socket_fd, SIOCGIFFLAGS, &ifr) < 0) { + ::close(m_socket_fd); + m_socket_fd = -1; + LOG(Log::ERR, CanLogIt::h()) << "Failed to get interface flags"; + return CanReturnCode::internal_api_error; + } + + if (!(ifr.ifr_flags & IFF_UP)) { + ::close(m_socket_fd); + m_socket_fd = -1; + LOG(Log::ERR, CanLogIt::h()) + << "CAN interface " << args().config.bus_name.value() << " is down"; + return CanReturnCode::unknown_open_error; // To be consistent with anagate + } + struct sockaddr_can addr; memset(&addr, 0, sizeof(addr)); addr.can_family = AF_CAN; - addr.can_ifindex = ifr.ifr_ifindex; + addr.can_ifindex = ifindex; if (bind(m_socket_fd, (struct sockaddr*)&addr, sizeof(addr)) < 0) { ::close(m_socket_fd); From 395d0a1b515cea76b8f65294bab44ec447a2d777 Mon Sep 17 00:00:00 2001 From: Tiago Lourinho Date: Mon, 14 Sep 2026 16:14:51 +0200 Subject: [PATCH 2/4] added troubleshooting section --- docs/README.md | 4 ++++ docs/Troubleshooting.md | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 docs/Troubleshooting.md diff --git a/docs/README.md b/docs/README.md index 2c9a91ec..04df4e5d 100644 --- a/docs/README.md +++ b/docs/README.md @@ -62,6 +62,10 @@ set( CAN_MODULE_URL https://github.com/quasar-team/CanModule.git ) clone_quasar_module( ${CAN_MODULE_URL} master ${CMAKE_CURRENT_SOURCE_DIR}/CanModule ) ``` +## Troubleshooting + +See [Troubleshooting.md](Troubleshooting.md) for known issues and workarounds. + ## Contact E-Mail: diff --git a/docs/Troubleshooting.md b/docs/Troubleshooting.md new file mode 100644 index 00000000..2383de81 --- /dev/null +++ b/docs/Troubleshooting.md @@ -0,0 +1,18 @@ +# Troubleshooting + +## Bitrate setting with vcan + +Setting the bitrate on a virtual CAN (`vcan`) interface will either be ignored or result in an +error, depending on the CAN device configuration: + +| `bus_name` | `bitrate` | `vcan` | Result | +| --- | --- | --- | --- | +| vcan0 | null | true | Works fine | +| vcan0 | defined | true | Works fine after running `sudo setcap cap_net_admin=ep /path/to/your/binary` (but bitrate is ignored) | +| vcan0 | null | false | Works fine | +| vcan0 | defined | false | Results in an error while trying to set the bitrate | + +## Non-deterministic can0/can1/... mapping with multiple Peak devices + +When multiple Peak devices are connected via USB, the mapping between the USB ports and the +resulting `can0`, `can1`, etc. interfaces is not deterministic. From 825d83d839b8a48a6d04122cbf23fa340f8ce134 Mon Sep 17 00:00:00 2001 From: Tiago Lourinho Date: Mon, 14 Sep 2026 17:02:13 +0200 Subject: [PATCH 3/4] added fail_open --- src/include/CanVendorSocketCan.h | 12 +++++++ src/main/CanVendorSocketCan.cpp | 57 ++++++++++++++++---------------- 2 files changed, 40 insertions(+), 29 deletions(-) diff --git a/src/include/CanVendorSocketCan.h b/src/include/CanVendorSocketCan.h index dae346b0..d3f49c4b 100644 --- a/src/include/CanVendorSocketCan.h +++ b/src/include/CanVendorSocketCan.h @@ -43,6 +43,18 @@ struct CanVendorSocketCan : CanDevice { static const CanFrame translate(const struct can_frame& message) noexcept; static struct can_frame translate(const CanFrame& frame) noexcept; + /** + * @brief Closes any open socket/epoll file descriptors, logs an error + * message, and returns an error code. + * + * @param message The error message to log. + * @param code The error code to return. + * + * @return CanReturnCode The given error code. + */ + CanReturnCode fail_open(const std::string& message, + CanReturnCode code) noexcept; + int m_socket_fd{-1}; // File descriptor for the SocketCAN device int m_epoll_fd{-1}; // File descriptor for the epoll instance std::thread m_subscriber_thread; // Thread for the subscriber loop diff --git a/src/main/CanVendorSocketCan.cpp b/src/main/CanVendorSocketCan.cpp index 7ad6ebc7..e85eab08 100644 --- a/src/main/CanVendorSocketCan.cpp +++ b/src/main/CanVendorSocketCan.cpp @@ -40,6 +40,21 @@ CanVendorSocketCan::CanVendorSocketCan(const CanDeviceArguments& args) throw std::invalid_argument("Missing required bus name"); } } + +CanReturnCode CanVendorSocketCan::fail_open(const std::string& message, + CanReturnCode code) noexcept { + if (m_epoll_fd >= 0) { + ::close(m_epoll_fd); + m_epoll_fd = -1; + } + if (m_socket_fd >= 0) { + ::close(m_socket_fd); + m_socket_fd = -1; + } + LOG(Log::ERR, CanLogIt::h()) << message; + return code; +} + /** * @brief Opens the SocketCAN device and sets up the necessary configurations. * @@ -95,10 +110,8 @@ CanReturnCode CanVendorSocketCan::vendor_open() noexcept { strcpy(ifr.ifr_name, // NOLINT: Recipe from Offical SocketCan documentation args().config.bus_name.value().c_str()); if (ioctl(m_socket_fd, SIOCGIFINDEX, &ifr) < 0) { - ::close(m_socket_fd); - m_socket_fd = -1; - LOG(Log::ERR, CanLogIt::h()) << "Failed to get interface index"; - return CanReturnCode::internal_api_error; + return fail_open("Failed to get interface index", + CanReturnCode::internal_api_error); } // ifr_ifindex and ifr_flags share the same union in struct ifreq, so save @@ -106,18 +119,14 @@ CanReturnCode CanVendorSocketCan::vendor_open() noexcept { const int ifindex = ifr.ifr_ifindex; if (ioctl(m_socket_fd, SIOCGIFFLAGS, &ifr) < 0) { - ::close(m_socket_fd); - m_socket_fd = -1; - LOG(Log::ERR, CanLogIt::h()) << "Failed to get interface flags"; - return CanReturnCode::internal_api_error; + return fail_open("Failed to get interface flags", + CanReturnCode::internal_api_error); } if (!(ifr.ifr_flags & IFF_UP)) { - ::close(m_socket_fd); - m_socket_fd = -1; - LOG(Log::ERR, CanLogIt::h()) - << "CAN interface " << args().config.bus_name.value() << " is down"; - return CanReturnCode::unknown_open_error; // To be consistent with anagate + return fail_open( + "CAN interface " + args().config.bus_name.value() + " is down", + CanReturnCode::unknown_open_error); // To be consistent with anagate } struct sockaddr_can addr; @@ -126,10 +135,8 @@ CanReturnCode CanVendorSocketCan::vendor_open() noexcept { addr.can_ifindex = ifindex; if (bind(m_socket_fd, (struct sockaddr*)&addr, sizeof(addr)) < 0) { - ::close(m_socket_fd); - m_socket_fd = -1; - LOG(Log::ERR, CanLogIt::h()) << "Failed to bind socket"; - return CanReturnCode::internal_api_error; + return fail_open("Failed to bind socket", + CanReturnCode::internal_api_error); } if (args().receiver != nullptr || args().on_error != nullptr) { @@ -137,11 +144,8 @@ CanReturnCode CanVendorSocketCan::vendor_open() noexcept { // Create epoll instance m_epoll_fd = epoll_create1(0); if (m_epoll_fd < 0) { - ::close(m_socket_fd); - m_socket_fd = -1; - LOG(Log::ERR, CanLogIt::h()) << "Failed to create epoll instance"; - - return CanReturnCode::internal_api_error; + return fail_open("Failed to create epoll instance", + CanReturnCode::internal_api_error); } // Add the socket to the epoll instance @@ -150,13 +154,8 @@ CanReturnCode CanVendorSocketCan::vendor_open() noexcept { ev.data.fd = m_socket_fd; if (epoll_ctl(m_epoll_fd, EPOLL_CTL_ADD, m_socket_fd, &ev) < 0) { - ::close(m_epoll_fd); - ::close(m_socket_fd); - m_epoll_fd = -1; - m_socket_fd = -1; - LOG(Log::ERR, CanLogIt::h()) << "Failed to add socket to epoll"; - - return CanReturnCode::internal_api_error; + return fail_open("Failed to add socket to epoll", + CanReturnCode::internal_api_error); } // Start the subscriber thread From a61dcbe152f455b8f85c6e316d4542716fbbbaef Mon Sep 17 00:00:00 2001 From: Luis Miguens Fernandez Date: Fri, 18 Sep 2026 10:20:43 +0200 Subject: [PATCH 4/4] Update Troubleshooting.md --- docs/Troubleshooting.md | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/docs/Troubleshooting.md b/docs/Troubleshooting.md index 2383de81..5541eacf 100644 --- a/docs/Troubleshooting.md +++ b/docs/Troubleshooting.md @@ -1,16 +1,11 @@ # Troubleshooting -## Bitrate setting with vcan +## Bitrate setting -Setting the bitrate on a virtual CAN (`vcan`) interface will either be ignored or result in an -error, depending on the CAN device configuration: +When you set the bitrate, CanModule will try to configure and start the socket. In the case +of Virtual SocketCAN (`vcan`), please set a dummy value for the bitrate and `vcan` to `true`. -| `bus_name` | `bitrate` | `vcan` | Result | -| --- | --- | --- | --- | -| vcan0 | null | true | Works fine | -| vcan0 | defined | true | Works fine after running `sudo setcap cap_net_admin=ep /path/to/your/binary` (but bitrate is ignored) | -| vcan0 | null | false | Works fine | -| vcan0 | defined | false | Results in an error while trying to set the bitrate | +When the bitrate is not set, CanModule assumes the socket is already configured and started. ## Non-deterministic can0/can1/... mapping with multiple Peak devices