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..5541eacf --- /dev/null +++ b/docs/Troubleshooting.md @@ -0,0 +1,13 @@ +# Troubleshooting + +## Bitrate setting + +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`. + +When the bitrate is not set, CanModule assumes the socket is already configured and started. + +## 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. 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 d6cf9dea..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,22 +110,33 @@ 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 + // 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) { + return fail_open("Failed to get interface flags", + CanReturnCode::internal_api_error); + } + + if (!(ifr.ifr_flags & IFF_UP)) { + 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; 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); - 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) { @@ -118,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 @@ -131,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