audio: add rollback support for streams in setAudioPatch.
This rollback support isn't altering the the definition
of the IModule::setAudioPatch API, in case of success.
But, in case of failure, setAudioPatch tries to rollback
to the old patch.
Although, rollback isn't guaranteed.
But the rollback attempt is ensured.
Thereby, rollback support enables framework to take fail safe
decisions and tries to create a deterministic behavior for HAL.
Add brief comments for this logic.
Reason, the Bluetooth devices (external devices) may fail to connect
and thus leaving the stream with empty devices may not be desirable.
Thus, Rollback attempt to previous devices may be desirable.
Bug: 370242708
Test: atest VtsHalAudioCoreTargetTest
Change-Id: I40627c6af1df61c6331cbadf32d259378906431c
diff --git a/audio/aidl/default/Module.cpp b/audio/aidl/default/Module.cpp
index a2a357c..51b6085 100644
--- a/audio/aidl/default/Module.cpp
+++ b/audio/aidl/default/Module.cpp
@@ -467,58 +467,132 @@
fillConnectionsHelper(connections, patch.sinkPortConfigIds, patch.sourcePortConfigIds);
} // Otherwise, there are no streams to notify.
};
- fillConnections(oldConnections, oldPatch);
- fillConnections(newConnections, newPatch);
-
- std::for_each(oldConnections.begin(), oldConnections.end(), [&](const auto& connectionPair) {
- const int32_t mixPortConfigId = connectionPair.first;
- if (auto it = newConnections.find(mixPortConfigId);
- it == newConnections.end() || it->second != connectionPair.second) {
- if (auto status = mStreams.setStreamConnectedDevices(mixPortConfigId, {});
- status.isOk()) {
- LOG(DEBUG) << "updateStreamsConnectedState: The stream on port config id "
- << mixPortConfigId << " has been disconnected";
- } else {
- // Disconnection is tricky to roll back, just register a failure.
- maybeFailure = std::move(status);
+ auto restoreOldConnections = [&](const std::set<int32_t>& mixPortIds,
+ const bool continueWithEmptyDevices) {
+ for (const auto mixPort : mixPortIds) {
+ if (auto it = oldConnections.find(mixPort);
+ continueWithEmptyDevices || it != oldConnections.end()) {
+ const std::vector<AudioDevice> d =
+ it != oldConnections.end() ? getDevicesFromDevicePortConfigIds(it->second)
+ : std::vector<AudioDevice>();
+ if (auto status = mStreams.setStreamConnectedDevices(mixPort, d); status.isOk()) {
+ LOG(WARNING) << ":updateStreamsConnectedState: rollback: mix port config:"
+ << mixPort
+ << (d.empty() ? "; not connected"
+ : std::string("; connected to ") +
+ ::android::internal::ToString(d));
+ } else {
+ // can't do much about rollback failures
+ LOG(ERROR)
+ << ":updateStreamsConnectedState: rollback: failed for mix port config:"
+ << mixPort;
+ }
}
}
- });
- if (!maybeFailure.isOk()) return maybeFailure;
- std::set<int32_t> idsToDisconnectOnFailure;
- std::for_each(newConnections.begin(), newConnections.end(), [&](const auto& connectionPair) {
- const int32_t mixPortConfigId = connectionPair.first;
- if (auto it = oldConnections.find(mixPortConfigId);
- it == oldConnections.end() || it->second != connectionPair.second) {
- const auto connectedDevices = getDevicesFromDevicePortConfigIds(connectionPair.second);
+ };
+ fillConnections(oldConnections, oldPatch);
+ fillConnections(newConnections, newPatch);
+ /**
+ * Illustration of oldConnections and newConnections
+ *
+ * oldConnections {
+ * a : {A,B,C},
+ * b : {D},
+ * d : {H,I,J},
+ * e : {N,O,P},
+ * f : {Q,R},
+ * g : {T,U,V},
+ * }
+ *
+ * newConnections {
+ * a : {A,B,C},
+ * c : {E,F,G},
+ * d : {K,L,M},
+ * e : {N,P},
+ * f : {Q,R,S},
+ * g : {U,V,W},
+ * }
+ *
+ * Expected routings:
+ * 'a': is ignored both in disconnect step and connect step,
+ * due to same devices both in oldConnections and newConnections.
+ * 'b': handled only in disconnect step with empty devices because 'b' is only present
+ * in oldConnections.
+ * 'c': handled only in connect step with {E,F,G} devices because 'c' is only present
+ * in newConnections.
+ * 'd': handled only in connect step with {K,L,M} devices because 'd' is also present
+ * in newConnections and it is ignored in disconnected step.
+ * 'e': handled only in connect step with {N,P} devices because 'e' is also present
+ * in newConnections and it is ignored in disconnect step. please note that there
+ * is no exclusive disconnection for device {O}.
+ * 'f': handled only in connect step with {Q,R,S} devices because 'f' is also present
+ * in newConnections and it is ignored in disconnect step. Even though stream is
+ * already connected with {Q,R} devices and connection happens with {Q,R,S}.
+ * 'g': handled only in connect step with {U,V,W} devices because 'g' is also present
+ * in newConnections and it is ignored in disconnect step. There is no exclusive
+ * disconnection with devices {T,U,V}.
+ *
+ * If, any failure, will lead to restoreOldConnections (rollback).
+ * The aim of the restoreOldConnections is to make connections back to oldConnections.
+ * Failures in restoreOldConnections aren't handled.
+ */
+
+ std::set<int32_t> idsToConnectBackOnFailure;
+ // disconnection step
+ for (const auto& [oldMixPortConfigId, oldDevicePortConfigIds] : oldConnections) {
+ if (auto it = newConnections.find(oldMixPortConfigId); it == newConnections.end()) {
+ idsToConnectBackOnFailure.insert(oldMixPortConfigId);
+ if (auto status = mStreams.setStreamConnectedDevices(oldMixPortConfigId, {});
+ status.isOk()) {
+ LOG(DEBUG) << __func__ << ": The stream on port config id " << oldMixPortConfigId
+ << " has been disconnected";
+ } else {
+ maybeFailure = std::move(status);
+ // proceed to rollback even on one failure
+ break;
+ }
+ }
+ }
+
+ if (!maybeFailure.isOk()) {
+ restoreOldConnections(idsToConnectBackOnFailure, false /*continueWithEmptyDevices*/);
+ LOG(WARNING) << __func__ << ": failed to disconnect from old patch. attempted rollback";
+ return maybeFailure;
+ }
+
+ std::set<int32_t> idsToRollbackOnFailure;
+ // connection step
+ for (const auto& [newMixPortConfigId, newDevicePortConfigIds] : newConnections) {
+ if (auto it = oldConnections.find(newMixPortConfigId);
+ it == oldConnections.end() || it->second != newDevicePortConfigIds) {
+ const auto connectedDevices = getDevicesFromDevicePortConfigIds(newDevicePortConfigIds);
+ idsToRollbackOnFailure.insert(newMixPortConfigId);
if (connectedDevices.empty()) {
// This is important as workers use the vector size to derive the connection status.
- LOG(FATAL) << "updateStreamsConnectedState: No connected devices found for port "
- "config id "
- << mixPortConfigId;
+ LOG(FATAL) << __func__ << ": No connected devices found for port config id "
+ << newMixPortConfigId;
}
- if (auto status = mStreams.setStreamConnectedDevices(mixPortConfigId, connectedDevices);
+ if (auto status =
+ mStreams.setStreamConnectedDevices(newMixPortConfigId, connectedDevices);
status.isOk()) {
- LOG(DEBUG) << "updateStreamsConnectedState: The stream on port config id "
- << mixPortConfigId << " has been connected to: "
+ LOG(DEBUG) << __func__ << ": The stream on port config id " << newMixPortConfigId
+ << " has been connected to: "
<< ::android::internal::ToString(connectedDevices);
} else {
maybeFailure = std::move(status);
- idsToDisconnectOnFailure.insert(mixPortConfigId);
+ // proceed to rollback even on one failure
+ break;
}
}
- });
+ }
+
if (!maybeFailure.isOk()) {
- LOG(WARNING) << __func__ << ": " << mType
- << ": Due to a failure, disconnecting streams on port config ids "
- << ::android::internal::ToString(idsToDisconnectOnFailure);
- std::for_each(idsToDisconnectOnFailure.begin(), idsToDisconnectOnFailure.end(),
- [&](const auto& portConfigId) {
- auto status = mStreams.setStreamConnectedDevices(portConfigId, {});
- (void)status.isOk(); // Can't do much about a failure here.
- });
+ restoreOldConnections(idsToConnectBackOnFailure, false /*continueWithEmptyDevices*/);
+ restoreOldConnections(idsToRollbackOnFailure, true /*continueWithEmptyDevices*/);
+ LOG(WARNING) << __func__ << ": failed to connect for new patch. attempted rollback";
return maybeFailure;
}
+
return ndk::ScopedAStatus::ok();
}