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/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: