Added troubleshooting section and made socketcan open fail early

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: <icecontrols.support@cern.ch>

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.

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

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,34 +110,42 @@ 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) {
     LOG(Log::DBG, CanLogIt::h()) << "Starting CAN subscriber thread";
     // 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