Simplify bus configuration.

Previous bus configuration struct was meant for flexibility, but it
turned out that the only dimension that flexibility would go was a
serial number parameter. Let's rotate that configuration matrix by 90
degrees and just go a straightforward route of discriminating against
interface type.

Test: VTS
Bug: 135918744
Change-Id: I08967d0f78c998b0582958eb51bd387f9dbe15fe
diff --git a/automotive/can/1.0/ICanController.hal b/automotive/can/1.0/ICanController.hal
index 0c6f53e..e32e593 100644
--- a/automotive/can/1.0/ICanController.hal
+++ b/automotive/can/1.0/ICanController.hal
@@ -27,51 +27,22 @@
  */
 interface ICanController {
     /**
-     * Type of an interface, a mean to express the domain of device address.
+     * Type of an interface, an equivalent to BusConfig::InterfaceId
+     * union discriminator. Defines a number of specific standard hardware
+     * families and a generic catch-all type of {@see INDEXED}.
      */
     enum InterfaceType : uint8_t {
-        /**
-         * Virtual SocketCAN interface.
-         *
-         * The address is an interface name, such as vcan0. If the interface
-         * doesn't exist, HAL server must create it.
-         *
-         * Valid InterfaceIdentifier types:
-         *  - address.
-         */
+        /** Virtual SocketCAN interface. */
         VIRTUAL,
 
-        /**
-         * Native SocketCAN interface.
-         *
-         * The address is an interface name, such as can0.
-         *
-         * Valid InterfaceIdentifier types:
-         *  - address;
-         *  - serialno.
-         */
+        /** Native SocketCAN interface. */
         SOCKETCAN,
 
-        /**
-         * Serial-based interface.
-         *
-         * The address is a patch to a device, such as /dev/ttyUSB0.
-         *
-         * Valid InterfaceIdentifier types:
-         *  - address;
-         *  - serialno.
-         */
+        /** Serial line CAN interface. */
         SLCAN,
 
-        /**
-         * Proprietary interface, specific to the hardware system Android
-         * is running on. Instead of using address field, the interface is
-         * addressed with 0-based index.
-         *
-         * Valid InterfaceIdentifier types:
-         *  - index
-         */
-        INDEXED
+        /** Proprietary, device-specific interface. */
+        INDEXED,
     };
 
     enum Result : uint8_t {
@@ -92,10 +63,10 @@
         NOT_SUPPORTED,
 
         /**
-         * Provided address (interface name, device path) doesn't exist or there
-         * is no device with a given serial no.
+         * Provided interface ID (index, name, device path) doesn't exist or
+         * there is no device with a given serial number.
          */
-        BAD_ADDRESS,
+        BAD_INTERFACE_ID,
 
         /** Provided bit rate is not supported by the hardware. */
         BAD_BITRATE,
@@ -106,49 +77,76 @@
      *
      * ISO TP and CAN FD are currently not supported.
      */
-    struct BusConfiguration {
+    struct BusConfig {
         /**
          * Name under which ICanBus HIDL service should be published.
          *
          * It must consist of only alphanumeric characters and underscore
          * (a-z, A-Z, 0-9, '_'), at least 1 and at most 32 characters long.
+         *
+         * This field is *not* meant to distinguish between hardware interfaces
+         * nor preselect parameters like bitrate. The only intended side-effect
+         * of changing it should be a different ICanBus HIDL service name and
+         * the HIDL service should make no assumptions on its contents.
          */
         string name;
 
         /**
-         * Type of the hardware (or virtual) CAN interface.
+         * Hardware interface configuration.
+         *
+         * This union's discriminator has an equivalent enum
+         * {@see InterfaceType} to express compatibility via
+         * getSupportedInterfaceTypes().
          */
-        InterfaceType iftype;
+        safe_union InterfaceId {
+            /** Virtual SocketCAN interface. */
+            struct Virtual {
+                /** Interface name, such as vcan0. If the interface doesn't
+                 * exist, HAL server must create it.
+                 */
+                string ifname;
+            } virtualif;
 
-        /**
-         * Identification of hardware interface to configure.
-         */
-        safe_union InterfaceIdentifier {
-            /**
-             * Interface name or other mean of identification of the specific
-             * interface port. Syntax depends on {@see iftype}, for details
-             * {@see InterfaceType}.
-             */
-            string address;
+            /** Native SocketCAN interface. */
+            safe_union Socketcan {
+                /** Interface name, such as can0. */
+                string ifname;
+                /**
+                 * Alternatively to providing {@see ifname}, one may provide a
+                 * list of interface serial number suffixes. If there happens to
+                 * be a device (like USB2CAN) with a matching serial number
+                 * suffix, the HAL service will have to select it.
+                 *
+                 * Client may utilize this in two ways: by matching against the
+                 * entire serial number, or the last few characters (usually
+                 * one). The former is better for small-scale test deployments
+                 * (with just a handful of vehicles), the latter is good for
+                 * larger scale (where a small suffix list may support large
+                 * test fleet).
+                 */
+                vec<string> serialno;
+            } socketcan;
+
+            /** Serial line CAN interface. */
+            safe_union Slcan {
+                /** Path to a device, such as /dev/ttyUSB0. */
+                string ttyname;
+                /**
+                 * List of interface serial number suffixes.
+                 * {@see Socketcan::serialno}
+                 */
+                vec<string> serialno;
+            } slcan;
 
             /**
-             * Numerical identifier of interface, used for InterfaceType#INDEXED.
-             */
-            uint8_t index;
-
-            /**
-             * Alternatively to providing {@see address}, one may provide a list
-             * of interface serial number suffixes. If there happens to be
-             * a device (like USB2CAN) with a matching serial number suffix,
-             * it gets selected.
+             * Proprietary, device-specific interface.
              *
-             * Client may utilize this in two ways: by matching against the
-             * entire serial number, or the last few characters (usually one).
-             * The former is better for small-scale test deployments (with just
-             * a handful of vehicles), the latter is good for larger scale
-             * (where a small suffix list may support large test fleet).
+             * Non-SocketCAN interfaces should use this variant.
              */
-            vec<string> serialno;
+            struct Indexed {
+                /** Interface number, 0-based. */
+                uint8_t index;
+            } indexed;
         } interfaceId;
 
         /**
@@ -156,7 +154,8 @@
          *
          * Typical bit rates are: 100000, 125000, 250000, 500000.
          *
-         * For virtual interfaces this value is ignored.
+         * For {@see interfaceId#virtual} and pre-configured
+         * {@see interfaceId#indexed} interfaces this value is ignored.
          */
         uint32_t bitrate;
     };
@@ -164,17 +163,17 @@
     /**
      * Fetches the list of interface types supported by this HAL server.
      *
-     * @return iftypes The list of supported interface types
+     * @return iftypes The list of supported interface types.
      */
     getSupportedInterfaceTypes() generates (vec<InterfaceType> iftypes);
 
     /**
      * Bring up the CAN interface and publish ICanBus server instance.
      *
-     * @param config Configuration of the CAN interface
+     * @param config Configuration of the CAN interface.
      * @return result OK if the operation succeeded; error code otherwise.
      */
-    upInterface(BusConfiguration config) generates (Result result);
+    upInterface(BusConfig config) generates (Result result);
 
     /**
      * Unpublish ICanBus server instance and bring down the CAN interface.
@@ -182,9 +181,9 @@
      * In case of failure, at least the ICanBus server instance must be
      * unpublished and resources freed on best-effort basis.
      *
-     * @param name Name of the interface (@see BusConfiguration#name} to
-     * bring down
-     * @return success true in case of success, false otherwise
+     * @param name Name of the interface (@see BusConfig#name} to
+     * bring down.
+     * @return success true in case of success, false otherwise.
      */
     downInterface(string name) generates (bool success);
 };
diff --git a/automotive/can/1.0/default/CanBus.cpp b/automotive/can/1.0/default/CanBus.cpp
index 8fb09eb..9f704c1 100644
--- a/automotive/can/1.0/default/CanBus.cpp
+++ b/automotive/can/1.0/default/CanBus.cpp
@@ -124,7 +124,7 @@
     if (!isUp.has_value()) {
         // preUp() should prepare the interface (either create or make sure it's there)
         LOG(ERROR) << "Interface " << mIfname << " didn't get prepared";
-        return ICanController::Result::BAD_ADDRESS;
+        return ICanController::Result::BAD_INTERFACE_ID;
     }
 
     if (!*isUp && !netdevice::up(mIfname)) {
diff --git a/automotive/can/1.0/default/CanBusNative.cpp b/automotive/can/1.0/default/CanBusNative.cpp
index ef04d01..aafbecc 100644
--- a/automotive/can/1.0/default/CanBusNative.cpp
+++ b/automotive/can/1.0/default/CanBusNative.cpp
@@ -28,7 +28,7 @@
 ICanController::Result CanBusNative::preUp() {
     if (!netdevice::exists(mIfname)) {
         LOG(ERROR) << "Interface " << mIfname << " doesn't exist";
-        return ICanController::Result::BAD_ADDRESS;
+        return ICanController::Result::BAD_INTERFACE_ID;
     }
 
     if (mBitrate == 0) {
diff --git a/automotive/can/1.0/default/CanBusSlcan.cpp b/automotive/can/1.0/default/CanBusSlcan.cpp
index 0feee8f..d15905d 100644
--- a/automotive/can/1.0/default/CanBusSlcan.cpp
+++ b/automotive/can/1.0/default/CanBusSlcan.cpp
@@ -81,7 +81,7 @@
     mFd = base::unique_fd(open(mUartName.c_str(), O_RDWR | O_NONBLOCK | O_NOCTTY));
     if (!mFd.ok()) {
         LOG(ERROR) << "SLCAN Failed to open " << mUartName << ": " << strerror(errno);
-        return ICanController::Result::BAD_ADDRESS;
+        return ICanController::Result::BAD_INTERFACE_ID;
     }
 
     // If the device is already up, update the iface name in our CanBusSlcan object
diff --git a/automotive/can/1.0/default/CanController.cpp b/automotive/can/1.0/default/CanController.cpp
index fb648c1..dd80402 100644
--- a/automotive/can/1.0/default/CanController.cpp
+++ b/automotive/can/1.0/default/CanController.cpp
@@ -27,7 +27,8 @@
 
 namespace android::hardware::automotive::can::V1_0::implementation {
 
-using IfaceIdDisc = ICanController::BusConfiguration::InterfaceIdentifier::hidl_discriminator;
+using IfId = ICanController::BusConfig::InterfaceId;
+using IfIdDisc = ICanController::BusConfig::InterfaceId::hidl_discriminator;
 
 Return<void> CanController::getSupportedInterfaceTypes(getSupportedInterfaceTypes_cb _hidl_cb) {
     _hidl_cb({ICanController::InterfaceType::VIRTUAL, ICanController::InterfaceType::SOCKETCAN,
@@ -40,8 +41,7 @@
     return std::regex_match(name, nameRE);
 }
 
-Return<ICanController::Result> CanController::upInterface(
-        const ICanController::BusConfiguration& config) {
+Return<ICanController::Result> CanController::upInterface(const ICanController::BusConfig& config) {
     LOG(VERBOSE) << "Attempting to bring interface up: " << toString(config);
 
     std::lock_guard<std::mutex> lck(mCanBusesGuard);
@@ -58,24 +58,23 @@
 
     sp<CanBus> busService;
 
-    if (config.iftype == ICanController::InterfaceType::SOCKETCAN) {
-        // TODO(b/135918744): support serialno
-        if (config.interfaceId.getDiscriminator() == IfaceIdDisc::address) {
-            busService = new CanBusNative(config.interfaceId.address(), config.bitrate);
+    if (config.interfaceId.getDiscriminator() == IfIdDisc::socketcan) {
+        // TODO(b/142654031): support serialno
+        auto& socketcan = config.interfaceId.socketcan();
+        if (socketcan.getDiscriminator() == IfId::Socketcan::hidl_discriminator::ifname) {
+            busService = new CanBusNative(socketcan.ifname(), config.bitrate);
         } else {
-            return ICanController::Result::BAD_ADDRESS;
+            return ICanController::Result::BAD_INTERFACE_ID;
         }
-    } else if (config.iftype == ICanController::InterfaceType::VIRTUAL) {
-        if (config.interfaceId.getDiscriminator() == IfaceIdDisc::address) {
-            busService = new CanBusVirtual(config.interfaceId.address());
+    } else if (config.interfaceId.getDiscriminator() == IfIdDisc::virtualif) {
+        busService = new CanBusVirtual(config.interfaceId.virtualif().ifname);
+    } else if (config.interfaceId.getDiscriminator() == IfIdDisc::slcan) {
+        // TODO(b/142654031): support serialno
+        auto& slcan = config.interfaceId.slcan();
+        if (slcan.getDiscriminator() == IfId::Slcan::hidl_discriminator::ttyname) {
+            busService = new CanBusSlcan(slcan.ttyname(), config.bitrate);
         } else {
-            return ICanController::Result::BAD_ADDRESS;
-        }
-    } else if (config.iftype == ICanController::InterfaceType::SLCAN) {
-        if (config.interfaceId.getDiscriminator() == IfaceIdDisc::address) {
-            busService = new CanBusSlcan(config.interfaceId.address(), config.bitrate);
-        } else {
-            return ICanController::Result::BAD_ADDRESS;
+            return ICanController::Result::BAD_INTERFACE_ID;
         }
     } else {
         return ICanController::Result::NOT_SUPPORTED;
diff --git a/automotive/can/1.0/default/CanController.h b/automotive/can/1.0/default/CanController.h
index 99a551a..27e82f3 100644
--- a/automotive/can/1.0/default/CanController.h
+++ b/automotive/can/1.0/default/CanController.h
@@ -25,8 +25,7 @@
 struct CanController : public ICanController {
     Return<void> getSupportedInterfaceTypes(getSupportedInterfaceTypes_cb _hidl_cb) override;
 
-    Return<ICanController::Result> upInterface(
-            const ICanController::BusConfiguration& config) override;
+    Return<ICanController::Result> upInterface(const ICanController::BusConfig& config) override;
     Return<bool> downInterface(const hidl_string& name) override;
 
   private:
diff --git a/automotive/can/1.0/tools/canhalctrl.cpp b/automotive/can/1.0/tools/canhalctrl.cpp
index 5494ba3..33755bf 100644
--- a/automotive/can/1.0/tools/canhalctrl.cpp
+++ b/automotive/can/1.0/tools/canhalctrl.cpp
@@ -71,15 +71,31 @@
         if (!isSupported(ctrl, type)) continue;
         anySupported = true;
 
-        ICanController::BusConfiguration config = {};
+        ICanController::BusConfig config = {};
         config.name = busName;
-        config.iftype = type;
         config.bitrate = bitrate;
 
-        if (type == ICanController::InterfaceType::INDEXED) {
-            config.interfaceId.index(std::stol(interface));
+        // TODO(b/146214370): move interfaceId constructors to a library
+        using IfCfg = ICanController::BusConfig::InterfaceId;
+        if (type == ICanController::InterfaceType::VIRTUAL) {
+            config.interfaceId.virtualif({interface});
+        } else if (type == ICanController::InterfaceType::SOCKETCAN) {
+            IfCfg::Socketcan socketcan = {};
+            socketcan.ifname(interface);
+            config.interfaceId.socketcan(socketcan);
+        } else if (type == ICanController::InterfaceType::SLCAN) {
+            IfCfg::Slcan slcan = {};
+            slcan.ttyname(interface);
+            config.interfaceId.slcan(slcan);
+        } else if (type == ICanController::InterfaceType::INDEXED) {
+            auto idx = std::stol(interface);
+            if (idx < 0 || idx > UINT8_MAX) {
+                std::cerr << "Interface index out of range: " << idx;
+                return -1;
+            }
+            config.interfaceId.indexed({uint8_t(idx)});
         } else {
-            config.interfaceId.address(interface);
+            CHECK(false) << "Unexpected interface type: " << toString(type);
         }
 
         const auto upresult = ctrl->upInterface(config);
diff --git a/automotive/can/1.0/vts/functional/VtsHalCanBusVirtualV1_0TargetTest.cpp b/automotive/can/1.0/vts/functional/VtsHalCanBusVirtualV1_0TargetTest.cpp
index efaad53..68d555d 100644
--- a/automotive/can/1.0/vts/functional/VtsHalCanBusVirtualV1_0TargetTest.cpp
+++ b/automotive/can/1.0/vts/functional/VtsHalCanBusVirtualV1_0TargetTest.cpp
@@ -81,7 +81,7 @@
 struct Bus {
     DISALLOW_COPY_AND_ASSIGN(Bus);
 
-    Bus(sp<ICanController> controller, const ICanController::BusConfiguration& config)
+    Bus(sp<ICanController> controller, const ICanController::BusConfig& config)
         : mIfname(config.name), mController(controller) {
         const auto result = controller->upInterface(config);
         EXPECT_EQ(ICanController::Result::OK, result);
@@ -122,6 +122,7 @@
 
     void send(const CanMessage& msg) {
         EXPECT_NE(mBus, nullptr);
+        if (!mBus) return;
         const auto result = mBus->send(msg);
         EXPECT_EQ(Result::OK, result);
     }
@@ -196,10 +197,9 @@
     const auto idx = mLastIface++;
     EXPECT_LT(idx, mBusNames.size());
 
-    ICanController::BusConfiguration config = {};
+    ICanController::BusConfig config = {};
     config.name = mBusNames[idx];
-    config.iftype = InterfaceType::VIRTUAL;
-    config.interfaceId.address("vcan50");
+    config.interfaceId.virtualif({"vcan50"});
 
     return Bus(mCanController, config);
 }
diff --git a/automotive/can/1.0/vts/functional/VtsHalCanControllerV1_0TargetTest.cpp b/automotive/can/1.0/vts/functional/VtsHalCanControllerV1_0TargetTest.cpp
index b2edd78..0c5d976 100644
--- a/automotive/can/1.0/vts/functional/VtsHalCanControllerV1_0TargetTest.cpp
+++ b/automotive/can/1.0/vts/functional/VtsHalCanControllerV1_0TargetTest.cpp
@@ -31,6 +31,7 @@
 
 using hardware::hidl_vec;
 using InterfaceType = ICanController::InterfaceType;
+using IfId = ICanController::BusConfig::InterfaceId;
 
 static utils::SimpleHidlEnvironment<ICanController>* gEnv = nullptr;
 
@@ -89,10 +90,23 @@
 
 bool CanControllerHalTest::up(InterfaceType iftype, std::string srvname, std::string ifname,
                               ICanController::Result expected) {
-    ICanController::BusConfiguration config = {};
+    ICanController::BusConfig config = {};
     config.name = srvname;
-    config.iftype = iftype;
-    config.interfaceId.address(ifname);
+
+    // TODO(b/146214370): move interfaceId constructors to a library
+    if (iftype == InterfaceType::SOCKETCAN) {
+        IfId::Socketcan socketcan = {};
+        socketcan.ifname(ifname);
+        config.interfaceId.socketcan(socketcan);
+    } else if (iftype == InterfaceType::SLCAN) {
+        IfId::Slcan slcan = {};
+        slcan.ttyname(ifname);
+        config.interfaceId.slcan(slcan);
+    } else if (iftype == InterfaceType::VIRTUAL) {
+        config.interfaceId.virtualif({ifname});
+    } else {
+        EXPECT_TRUE(false) << "Unexpected iftype: " << toString(iftype);
+    }
 
     const auto upresult = mCanController->upInterface(config);
 
@@ -155,54 +169,64 @@
     assertRegistered(name, false);
 }
 
-TEST_F(CanControllerHalTest, IdentifierCompatibility) {
-    using IdDisc = ICanController::BusConfiguration::InterfaceIdentifier::hidl_discriminator;
-    static const std::map<InterfaceType, std::vector<IdDisc>> compatMatrix = {
-            {InterfaceType::VIRTUAL, {IdDisc::address}},
-            {InterfaceType::SOCKETCAN, {IdDisc::address, IdDisc::serialno}},
-            {InterfaceType::SLCAN, {IdDisc::address, IdDisc::serialno}},
-            {InterfaceType::INDEXED, {IdDisc::index}},
+TEST_F(CanControllerHalTest, ConfigCompatibility) {
+    // using random-ish addresses, which may not be valid - we can't test the success case
+    // TODO(b/146214370): move interfaceId constructors to a library
+    IfId virtualCfg = {};
+    virtualCfg.virtualif({"vcan70"});
+
+    IfId::Socketcan socketcanIfname = {};
+    socketcanIfname.ifname("can0");
+    IfId socketcanIfnameCfg = {};
+    socketcanIfnameCfg.socketcan(socketcanIfname);
+
+    IfId::Socketcan socketcanSerial = {};
+    socketcanSerial.serialno({"1234", "2345"});
+    IfId socketcanSerialCfg = {};
+    socketcanSerialCfg.socketcan(socketcanSerial);
+
+    IfId::Slcan slcanTtyname = {};
+    slcanTtyname.ttyname("/dev/ttyUSB0");
+    IfId slcanTtynameCfg = {};
+    slcanTtynameCfg.slcan(slcanTtyname);
+
+    IfId::Slcan slcanSerial = {};
+    slcanSerial.serialno({"dead", "beef"});
+    IfId slcanSerialCfg = {};
+    slcanSerialCfg.slcan(slcanSerial);
+
+    IfId indexedCfg = {};
+    indexedCfg.indexed({0});
+
+    static const std::vector<std::pair<InterfaceType, IfId>> compatMatrix = {
+            {InterfaceType::VIRTUAL, virtualCfg},
+            {InterfaceType::SOCKETCAN, socketcanIfnameCfg},
+            {InterfaceType::SOCKETCAN, socketcanSerialCfg},
+            {InterfaceType::SLCAN, slcanTtynameCfg},
+            {InterfaceType::SLCAN, slcanSerialCfg},
+            {InterfaceType::INDEXED, indexedCfg},
     };
-    static const std::vector<IdDisc> allDisc = {IdDisc::address, IdDisc::index, IdDisc::serialno};
 
-    for (const auto [iftype, supported] : compatMatrix) {
-        for (const auto iddisc : allDisc) {
-            LOG(INFO) << "Compatibility testing: " << iftype << " / " << iddisc;
+    for (const auto [iftype, cfg] : compatMatrix) {
+        LOG(INFO) << "Compatibility testing: " << iftype << " / " << cfg;
 
-            ICanController::BusConfiguration config = {};
-            config.name = "compattestsrv";
-            config.iftype = iftype;
-            config.bitrate = 125000;
+        ICanController::BusConfig config = {};
+        config.name = "compattestsrv";
+        config.bitrate = 125000;
+        config.interfaceId = cfg;
 
-            // using random-ish addresses, which may not be valid - we can't test the success case
-            if (iddisc == IdDisc::address) {
-                config.interfaceId.address("can0");
-            } else if (iddisc == IdDisc::index) {
-                config.interfaceId.index(0);
-            } else if (iddisc == IdDisc::serialno) {
-                config.interfaceId.serialno({"dummy", "dummier"});
-            }
+        const auto upresult = mCanController->upInterface(config);
 
-            const auto upresult = mCanController->upInterface(config);
+        if (!isSupported(iftype)) {
+            ASSERT_EQ(ICanController::Result::NOT_SUPPORTED, upresult);
+            continue;
+        }
+        ASSERT_NE(ICanController::Result::NOT_SUPPORTED, upresult);
 
-            if (!isSupported(iftype)) {
-                ASSERT_EQ(ICanController::Result::NOT_SUPPORTED, upresult);
-                continue;
-            }
-            ASSERT_NE(ICanController::Result::NOT_SUPPORTED, upresult);
-
-            bool isSupportedDisc =
-                    std::find(supported.begin(), supported.end(), iddisc) != supported.end();
-            if (!isSupportedDisc) {
-                ASSERT_EQ(ICanController::Result::BAD_ADDRESS, upresult);
-                continue;
-            }
-
-            if (upresult == ICanController::Result::OK) {
-                const auto dnresult = mCanController->downInterface(config.name);
-                ASSERT_TRUE(dnresult);
-                continue;
-            }
+        if (upresult == ICanController::Result::OK) {
+            const auto dnresult = mCanController->downInterface(config.name);
+            ASSERT_TRUE(dnresult);
+            continue;
         }
     }
 }
@@ -232,7 +256,9 @@
     const std::string name = mBusNames[0];
 
     assertRegistered(name, false);
-    if (!up(InterfaceType::VIRTUAL, name, "", ICanController::Result::BAD_ADDRESS)) GTEST_SKIP();
+    if (!up(InterfaceType::VIRTUAL, name, "", ICanController::Result::BAD_INTERFACE_ID)) {
+        GTEST_SKIP();
+    }
     assertRegistered(name, false);
 }
 
@@ -240,10 +266,30 @@
     const std::string name = mBusNames[0];
 
     assertRegistered(name, false);
-    if (!up(InterfaceType::SOCKETCAN, name, "can87", ICanController::Result::BAD_ADDRESS)) {
+    if (!up(InterfaceType::SOCKETCAN, name, "can87", ICanController::Result::BAD_INTERFACE_ID)) {
         GTEST_SKIP();
     }
     assertRegistered(name, false);
+
+    auto supported =
+            up(InterfaceType::SOCKETCAN, name, "", ICanController::Result::BAD_INTERFACE_ID);
+    ASSERT_TRUE(supported);
+    assertRegistered(name, false);
+}
+
+TEST_F(CanControllerHalTest, FailBadSlcanAddress) {
+    const std::string name = mBusNames[0];
+
+    assertRegistered(name, false);
+    if (!up(InterfaceType::SLCAN, name, "/dev/shouldnotexist123",
+            ICanController::Result::BAD_INTERFACE_ID)) {
+        GTEST_SKIP();
+    }
+    assertRegistered(name, false);
+
+    auto supported = up(InterfaceType::SLCAN, name, "", ICanController::Result::BAD_INTERFACE_ID);
+    ASSERT_TRUE(supported);
+    assertRegistered(name, false);
 }
 
 }  // namespace android::hardware::automotive::can::V1_0::vts
diff --git a/automotive/can/1.0/vts/utils/include/can-vts-utils/can-hal-printers.h b/automotive/can/1.0/vts/utils/include/can-vts-utils/can-hal-printers.h
index 3c30744..383b54c 100644
--- a/automotive/can/1.0/vts/utils/include/can-vts-utils/can-hal-printers.h
+++ b/automotive/can/1.0/vts/utils/include/can-vts-utils/can-hal-printers.h
@@ -35,8 +35,7 @@
 
 DEFINE_CAN_HAL_PRINTER(CanMessage, toString)
 DEFINE_CAN_HAL_PRINTER(ErrorEvent, toString)
-DEFINE_CAN_HAL_PRINTER_SIMPLE(
-        ICanController::BusConfiguration::InterfaceIdentifier::hidl_discriminator, int)
+DEFINE_CAN_HAL_PRINTER(ICanController::BusConfig::InterfaceId, toString);
 DEFINE_CAN_HAL_PRINTER(ICanController::InterfaceType, toString)
 DEFINE_CAN_HAL_PRINTER(ICanController::Result, toString)
 DEFINE_CAN_HAL_PRINTER(Result, toString)