Merge branch 'dev'

CHANGELOG-avdecc.md

@@ -4,6 +4,8 @@ All notable changes to the Avdecc Library will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [3.4.1] - 2023-01-11
+
 ## [3.4.0] - 2023-01-05
 ### Added
 - Checking validity of EntityModel given to a ControllerEntity or AggregateEntity

CHANGELOG-controller.md

@@ -4,6 +4,11 @@ All notable changes to the Avdecc Controller Library will be documented in this
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [3.4.1] - 2023-01-11
+### Fixed
+- [Crash when trying to get a ControlledEntity during OnPreAdvertise events](https://github.com/L-Acoustics/avdecc/issues/125)
+- [Sometimes incorrectly detecting loss of unsol for Milan devices](https://github.com/L-Acoustics/avdecc/issues/126)
+
 ## [3.4.0] - 2023-01-05
 ### Added
 - Checking validity of EntityModel given when creating a controller

CMakeLists.txt

@@ -10,9 +10,9 @@ cmake_minimum_required(VERSION 3.22)
 # It should have 3 digits (x.y.z) on main branch, and 4 digits (x.y.z.w) on dev and task branches. The last digit being used as beta label.
 # As soon as an API change occurs in dev or task branch, update the version number by following SemVer rules (so the API breaking changes are not lost/forgotten).
 # Example: 1.5.0 is the upcoming release version (main branch) of current 1.5.0.3 devel version (dev/task) which will be labelled 1.5.0-beta3
-set(LA_AVDECC_VERSION 3.4.0)
-set(LA_AVDECC_CONTROLLER_VERSION 3.4.0)
-set(LA_AVDECC_C_VERSION 3.4.0)
+set(LA_AVDECC_VERSION 3.4.1)
+set(LA_AVDECC_CONTROLLER_VERSION 3.4.1)
+set(LA_AVDECC_C_VERSION 3.4.1)
 
 ############ Override from command line "CMake -D<OPTION>=TRUE/FALSE/0/1/ON/OFF"
 

src/controller/avdeccControlledEntityImpl.cpp

@@ -2269,11 +2269,8 @@ void ControlledEntityImpl::setSubscribedToUnsolicitedNotifications(bool const is
 {
 	_isSubscribedToUnsolicitedNotifications = isSubscribed;
 
-	if (isSubscribed && _milanInfo && _milanInfo->protocolVersion == 1)
-	{
-		_expectedSequenceID = protocol::AecpSequenceID{ 0u };
-	}
-	else
+	// If unsubscribing, reset the expected sequence id
+	if (!isSubscribed)
 	{
 		_expectedSequenceID = std::nullopt;
 	}
@@ -2317,10 +2314,16 @@ ControlledEntity::Diagnostics& ControlledEntityImpl::getDiagnostics() noexcept
 bool ControlledEntityImpl::hasLostUnsolicitedNotification(protocol::AecpSequenceID const sequenceID) noexcept
 {
 	auto unmatched = false;
-	if (_expectedSequenceID.has_value())
+	if (_isSubscribedToUnsolicitedNotifications && _milanInfo && _milanInfo->protocolVersion == 1)
 	{
-		// Compare received sequenceID and expected one
-		unmatched = *_expectedSequenceID != sequenceID;
+		// Compare received sequenceID and expected one, if it's not the first one received.
+		// We don't expect 0 as first value, since the controller itself might restart (with the same entityID) without properly deregistering first,
+		// in which case the entity will send unsolicited continuing the previous sequence.
+		if (_expectedSequenceID.has_value())
+		{
+			unmatched = *_expectedSequenceID != sequenceID;
+		}
+		// Update next expected sequence ID
 		_expectedSequenceID = static_cast<protocol::AecpSequenceID>(sequenceID + 1u);
 	}
 	return unmatched;

src/controller/avdeccControllerImpl.hpp

@@ -711,7 +711,7 @@ class ControllerImpl final : public Controller, private entity::controller::Dele
 	/* ************************************************************ */
 	/* Private members                                              */
 	/* ************************************************************ */
-	mutable std::mutex _lock{}; // A mutex to protect all sensitive data members
+	mutable std::recursive_mutex _lock{}; // A mutex to protect all sensitive data members
 	ControlledEntityImpl::LockInformation::SharedPointer _entitiesSharedLockInformation{ std::make_shared<ControlledEntityImpl::LockInformation>() }; // The SharedLockInformation to be used by all managed ControlledEntities
 	std::unordered_map<UniqueIdentifier, SharedControlledEntityImpl, UniqueIdentifier::hash> _controlledEntities;
 	EndStation::UniquePointer _endStation{ nullptr, nullptr };

tests/src/controller/avdeccController_tests.cpp

@@ -2105,6 +2105,62 @@ TEST_F(MediaClockModel_F, StreamInput_Connected_Online_SwitchClockSource)
 	}
 }
 
+// Test for #125
+TEST_F(MediaClockModel_F, NotCrashing_Issue125)
+{
+	loadEntityFile("data/MediaClockModel/Entity_0x01.json");
+
+	try
+	{
+		auto& c = getController();
+		{
+			auto const& e = *c.getControlledEntityGuard(Entity01);
+			auto const& node = e.getClockDomainNode(la::avdecc::entity::model::ConfigurationIndex{ 0u }, la::avdecc::entity::model::ClockDomainIndex{ 0u });
+			ASSERT_EQ(2u, node.mediaClockChain.size());
+			{
+				auto const& n = node.mediaClockChain[0];
+				EXPECT_EQ(01, n.entityID);
+				EXPECT_EQ(la::avdecc::entity::model::ClockDomainIndex{ 0u }, n.clockDomainIndex);
+				EXPECT_EQ(la::avdecc::entity::model::ClockSourceIndex{ 0u }, n.clockSourceIndex);
+				EXPECT_EQ(la::avdecc::controller::model::MediaClockChainNode::Type::StreamInput, n.type);
+				EXPECT_EQ(la::avdecc::controller::model::MediaClockChainNode::Status::Active, n.status);
+				EXPECT_EQ(la::avdecc::entity::model::StreamIndex{ 0u }, n.streamInputIndex);
+				EXPECT_EQ(std::nullopt, n.streamOutputIndex);
+			}
+			{
+				auto const& n = node.mediaClockChain[1];
+				EXPECT_EQ(Entity11, n.entityID);
+				EXPECT_EQ(la::avdecc::entity::model::getInvalidDescriptorIndex(), n.clockDomainIndex);
+				EXPECT_EQ(la::avdecc::entity::model::getInvalidDescriptorIndex(), n.clockSourceIndex);
+				EXPECT_EQ(la::avdecc::controller::model::MediaClockChainNode::Type::Undefined, n.type);
+				EXPECT_EQ(la::avdecc::controller::model::MediaClockChainNode::Status::EntityOffline, n.status);
+				EXPECT_EQ(std::nullopt, n.streamInputIndex);
+				EXPECT_EQ(la::avdecc::entity::model::StreamIndex{ 0u }, n.streamOutputIndex);
+			}
+		}
+
+		// Expect Controller::Observer::onMediaClockChainChanged() to be called
+		registerMockObserver();
+		EXPECT_CALL(*this, onMediaClockChainChanged(::testing::_, c.getControlledEntityGuard(Entity01).get(), la::avdecc::entity::model::ClockDomainIndex{ 0u }, ::testing::_))
+			.WillOnce(testing::Invoke(
+				[](la::avdecc::controller::Controller const* const controller, la::avdecc::controller::ControlledEntity const* const entity, la::avdecc::entity::model::ClockDomainIndex const /*clockDomainIndex*/, la::avdecc::controller::model::MediaClockChain const& /*mcChain*/)
+				{
+					// Try to get a ControlledEntity inside the handler should not crash
+					auto const entityID = entity->getEntity().getEntityID();
+					auto const c = controller->getControlledEntityGuard(entityID);
+					// Dummy code to force variables
+					EXPECT_EQ(entityID, c->getEntity().getEntityID());
+				}));
+
+		// Entity coming online
+		loadEntityFile("data/MediaClockModel/Entity_0x11.json");
+	}
+	catch (...)
+	{
+		ASSERT_FALSE(true) << "Should not throw";
+	}
+}
+
 TEST(Controller, HashEntityModel)
 {
 	auto const flags = la::avdecc::entity::model::jsonSerializer::Flags{ la::avdecc::entity::model::jsonSerializer::Flag::ProcessADP, la::avdecc::entity::model::jsonSerializer::Flag::ProcessDynamicModel, la::avdecc::entity::model::jsonSerializer::Flag::ProcessStaticModel };