Pins/TIDAL: Add ability to refresh pins & refresh TIDAL mix pins

- Adds the ability for implementers to provide an `IPinRefresher` instance which will be called periodically to check if a given pin's metadata has changed. This can be used to update metadata on changing pins (like TIDAL Mixes) as well as detect and report back pins that may have changed or have stopped working after a period of time. Pins are currently refreshed: - 5mins after boot - When invoked - Every 24hours - Adds the ability for us to refresh TIDAL Mix pins. This will ensure we have the most up-to-date artwork available for those pins.
merakiacoustic · Jan 30, 2024 · a043a8f · a043a8f
1 parent 0896eaa
commit a043a8f
Show file tree

Hide file tree

Showing 7 changed files with 427 additions and 13 deletions.
diff --git a/OpenHome/Av/MediaPlayer.cpp b/OpenHome/Av/MediaPlayer.cpp
@@ -235,7 +235,8 @@ MediaPlayer::MediaPlayer(Net::DvStack& aDvStack, Net::CpStack& aCpStack, Net::Dv
 
     TUint maxDevicePins;
     if (aInitParams->PinsEnabled(maxDevicePins)) {
-        iPinsManager = new PinsManager(aReadWriteStore, maxDevicePins);
+        TimerFactory tf(iDvStack.Env());
+        iPinsManager = new PinsManager(aReadWriteStore, maxDevicePins, *iThreadPool, tf);
         iProviderPins = new ProviderPins(aDevice, aDvStack.Env(), *iPinsManager);
         iProduct->AddAttribute("Pins");
 

diff --git a/OpenHome/Av/Pins/Pins.cpp b/OpenHome/Av/Pins/Pins.cpp
@@ -11,6 +11,7 @@
 #include <OpenHome/Private/Parser.h>
 #include <OpenHome/Private/Uri.h>
 #include <OpenHome/Private/Converter.h>
+#include <OpenHome/Media/Debug.h>
 
 #include <algorithm>
 #include <iterator>
@@ -425,8 +426,9 @@ inline IPinsAccount& PinsManager::AccountSetter()
     return *iAccountSetter;
 }
 
-PinsManager::PinsManager(Configuration::IStoreReadWrite& aStore, TUint aMaxDevice)
-    : iStore(aStore)
+PinsManager::PinsManager(Configuration::IStoreReadWrite& aStore, TUint aMaxDevice, IThreadPool& aThreadPool, ITimerFactory& aTimerFactory, TUint aStartupRefreshDelay, TUint aRefreshPeriod)
+    : iRefreshPeriod(aRefreshPeriod)
+    , iStore(aStore)
     , iLock("Pin1")
     , iLockInvoke("Pin2")
     , iLockInvoker("Pin3")
@@ -437,8 +439,13 @@ PinsManager::PinsManager(Configuration::IStoreReadWrite& aStore, TUint aMaxDevic
     , iAccountSetter(nullptr)
     , iPinSetObserver(nullptr)
     , iInvoke(iIdProvider)
+    , iUpdated(iIdProvider)
     , iCurrent(nullptr)
 {
+    iRefreshTimer = aTimerFactory.CreateTimer(MakeFunctor(*this, &PinsManager::RefreshAll), "PinsManager-RefreshTask");
+    iRefreshTaskHandle = aThreadPool.CreateHandle(MakeFunctor(*this, &PinsManager::RefreshTask), "Pins-RefreshTask", ThreadPoolPriority::Low);
+
+    iRefreshTimer->FireIn(aStartupRefreshDelay);
 }
 
 PinsManager::~PinsManager()
@@ -449,10 +456,16 @@ PinsManager::~PinsManager()
             iCurrent->Cancel();
         }
     }
+
     iSemInvokerComplete.Wait();
     for (auto kvp : iInvokers) {
         delete kvp.second;
     }
+
+    iRefreshTaskHandle->Cancel();
+    iRefreshTaskHandle->Destroy();
+
+    delete iRefreshTimer;
 }
 
 void PinsManager::SetAccount(IPinsAccount& aAccount, TUint aCount)
@@ -481,6 +494,14 @@ void PinsManager::Add(IPinInvoker* aInvoker)
     }
 }
 
+void PinsManager::Add(IPinMetadataRefresher* aRefresher)
+{
+    AutoMutex m(iLock);
+    Brn mode(aRefresher->Mode());
+    ASSERT(iRefreshers.find(mode) == iRefreshers.end());
+    iRefreshers.insert(std::pair<Brn, IPinMetadataRefresher*>(mode, aRefresher));
+}
+
 void PinsManager::SetObserver(IPinsObserver& aObserver)
 {
     AutoMutex _(iLock);
@@ -675,6 +696,9 @@ void PinsManager::BeginInvoke()
             THROW(PinModeNotSupported);
         }
         invoker = it->second;
+
+        // Enqueue a request to refresh the pin metadata
+        iRefreshRequests.push(iInvoke.Id());
     }
     {
         AutoMutex __(iLockInvoker);
@@ -684,6 +708,7 @@ void PinsManager::BeginInvoke()
     }
     iSemInvokerComplete.Wait();
     iCurrent = invoker;
+
     Functor complete = MakeFunctor(*this, &PinsManager::NotifyInvocationCompleted);
     if (iPinSetObserver != nullptr) {
         TUint index = 0;
@@ -692,6 +717,9 @@ void PinsManager::BeginInvoke()
         }
     }
     iCurrent->BeginInvoke(iInvoke, complete);
+
+    // Fire the refresh handle to run in the background
+    iRefreshTaskHandle->TrySchedule();
 }
 
 void PinsManager::NotifyInvocationCompleted()
@@ -718,6 +746,135 @@ TBool PinsManager::TryGetIndexFromId(TUint aId, TUint& aIndex)
     return true;
 }
 
+void PinsManager::RefreshAll()
+{
+    {
+        AutoMutex m(iLock);
+        for(TUint id : iPinsDevice.IdArray()) {
+            if (id != IPinIdProvider::kIdEmpty) {
+                iRefreshRequests.push(id);
+            }
+        }
+        for(TUint id: iPinsAccount.IdArray()) {
+            if (id != IPinIdProvider::kIdEmpty) {
+                iRefreshRequests.push(id);
+            }
+        }
+    }
+
+    iRefreshTaskHandle->TrySchedule();
+
+    iRefreshTimer->Cancel();
+    iRefreshTimer->FireIn(iRefreshPeriod);
+}
+
+void PinsManager::RefreshTask()
+{
+    TBool scheduleAgain = true;
+    {
+        AutoMutex m(iLock);
+        scheduleAgain = DoRefreshPinsLocked();
+    }
+
+    if (scheduleAgain) {
+        iRefreshTaskHandle->TrySchedule();
+    }
+}
+
+TBool PinsManager::DoRefreshPinsLocked()
+{
+    static const TBool kStopRefreshing    = false;
+    static const TBool kTryRefreshNextPin = true;
+
+    if (iRefreshRequests.empty()) {
+        LOG_TRACE(kMedia, "PinsManager::RefreshPins - No more work required.\n");
+        return kStopRefreshing;
+    }
+
+    TUint pinIndex  = 0;
+    const IPin* pin = nullptr;
+
+    // Collect the ID of the pin we're hoping to refresh...
+    TUint pinIdToRefresh = iRefreshRequests.front();
+    iRefreshRequests.pop();
+
+    // Attempt to resolve this to a stored pin...
+    if (IsAccountId(pinIdToRefresh)) {
+        if (iPinsAccount.Contains(pinIdToRefresh)) {
+            pin = &iPinsAccount.PinFromId(pinIdToRefresh);
+            pinIndex = iPinsAccount.IndexFromId(pinIdToRefresh);
+        }
+    }
+    else {
+        if (iPinsDevice.Contains(pinIdToRefresh)) {
+            pin = &iPinsDevice.PinFromId(pinIdToRefresh);
+            pinIndex = iPinsDevice.IndexFromId(pinIdToRefresh);
+        }
+    }
+
+    // If the pin can't be found (likely updated before we've had a chance to process things) we'll try again with the next one
+    if (pin == nullptr) {
+        // Pin can't be found.
+        LOG_ERROR(kMedia, "PinsManager::RefreshTask - Requested refresh on ID: %u, but tht pin couldn't be found.\n", pinIdToRefresh);
+        return kTryRefreshNextPin;
+    }
+
+    IPinMetadataRefresher* refresher = nullptr;
+
+    Brn mode(pin->Mode());
+    if (mode.Bytes() == 0) {
+        LOG_ERROR(kMedia, "PinsManager::RefreshTask - ID: %u - No mode provided\n", pinIdToRefresh);
+        return kTryRefreshNextPin;
+    }
+    auto it = iRefreshers.find(mode);
+    if (it == iRefreshers.end()) {
+        LOG_INFO(kMedia, "PinsManager::RefreshTask - No refresher available for pin ID: %u (Mode: %.*s)\n", pinIdToRefresh, PBUF(mode));
+        return kTryRefreshNextPin;
+    }
+    refresher = it->second;
+
+    // If we've reached this point, the refresher MUST be set
+    ASSERT(refresher);
+
+    // Clear any previous data from the updated pin and request our refresher does the job...
+    iUpdated.Clear();
+    EPinMetadataStatus result = refresher->RefreshPinMetadata(*pin, iUpdated);
+    switch(result) {
+        case EPinMetadataStatus::Same: {
+            LOG_TRACE(kMedia, "PinsManager::RefreshTask - ID: %u : Refresher indicated that the metadata is unchanged.\n", pinIdToRefresh);
+            break;
+        }
+        case EPinMetadataStatus::Changed: {
+            LOG_INFO(kMedia, "PinsManager::RefreshTask - ID: %u : Refresher indicated that the metadata has changed.\n", pinIdToRefresh);
+
+            // NOTE: Can't call 'Set' directly here as that locks itself internally. Without this we'll end up with a recursive lock being taken.
+            if (IsAccountId(pinIdToRefresh)) {
+                AccountSetter().Set(pinIndex, iUpdated.Mode(), iUpdated.Type(), iUpdated.Uri(), iUpdated.Title(), iUpdated.Description(), iUpdated.ArtworkUri(), iUpdated.Shuffle());
+            }
+            else {
+                if (iPinsDevice.Set(pinIndex, iUpdated.Mode(), iUpdated.Type(), iUpdated.Uri(), iUpdated.Title(), iUpdated.Description(), iUpdated.ArtworkUri(), iUpdated.Shuffle())) {
+                    if (iObserver != nullptr) {
+                        iObserver->NotifyUpdatesDevice(iPinsDevice.IdArray());
+                    }
+                }
+            }
+            break;
+        }
+        case EPinMetadataStatus::Unresolvable: {
+            LOG_ERROR(kMedia, "PinsManager::RefreshTask - ID: %u : Refresher indicated that metadata could not be resolved. Perhaps the pinned item is no longer available?\n", pinIdToRefresh);
+            break;
+        }
+        case EPinMetadataStatus::Error: {
+            LOG_ERROR(kMedia, "PinsManager::RefreshTask - ID: %u : Refresher encountered an error when trying to refresh the pin.\n", pinIdToRefresh);
+            break;
+        }
+    }
+
+    return kTryRefreshNextPin;
+}
+
+
+
 static const Brn TryFindQueryValue(const Brx& aUri, const Brx& queryKey)
 {
     Parser parser(aUri);

diff --git a/OpenHome/Av/Pins/Pins.h b/OpenHome/Av/Pins/Pins.h
@@ -4,22 +4,25 @@
 #include <OpenHome/Buffer.h>
 #include <OpenHome/Exception.h>
 #include <OpenHome/Functor.h>
+#include <OpenHome/ThreadPool.h>
 #include <OpenHome/Private/Stream.h>
 #include <OpenHome/Private/Thread.h>
+#include <OpenHome/Private/Timer.h>
 
 #include <map>
+#include <queue>
 #include <vector>
 
 EXCEPTION(PinError)
-EXCEPTION(PinInvokeError);
+EXCEPTION(PinInvokeError)
 EXCEPTION(PinIndexOutOfRange)
 EXCEPTION(PinIdNotFound)
-EXCEPTION(PinModeNotSupported);
-EXCEPTION(PinTypeNotSupported);
-EXCEPTION(PinUriError);
-EXCEPTION(PinNothingToPlay);
-EXCEPTION(PinUriMissingRequiredParameter);
-EXCEPTION(PinInterrupted);
+EXCEPTION(PinModeNotSupported)
+EXCEPTION(PinTypeNotSupported)
+EXCEPTION(PinUriError)
+EXCEPTION(PinNothingToPlay)
+EXCEPTION(PinUriMissingRequiredParameter)
+EXCEPTION(PinInterrupted)
 
 namespace OpenHome {
     class WriterJsonObject;
@@ -206,11 +209,28 @@ class IPinInvoker
     virtual TBool SupportsVersion(TUint version) const = 0;
 };
 
+enum class EPinMetadataStatus
+{
+    Same,         // Metadata is unchanged
+    Changed,      // Something about the metadata has changed
+    Unresolvable, // The pinned item could not be resolved to an item
+    Error,        // Something went wrong when trying to get the metadata for an item
+};
+
+class IPinMetadataRefresher
+{
+public:
+    virtual ~IPinMetadataRefresher() {}
+    virtual const TChar* Mode() const = 0;
+    virtual EPinMetadataStatus RefreshPinMetadata(const IPin& aPin, Pin& aChangedPin) = 0;
+};
+
 class IPinsInvocable
 {
 public:
     virtual ~IPinsInvocable() {}
     virtual void Add(IPinInvoker* aInvoker) = 0; // transfers ownership
+    virtual void Add(IPinMetadataRefresher* aRefresher) = 0; // transfers ownership
 };
 
 class IPinsAccountStore
@@ -240,14 +260,23 @@ class PinsManager : public IPinsManager
                   , public IPinSetObservable
                   , private IPinsAccountObserver
 {
+    static const TUint kStartupRefreshDelay = 1000 * 60 * 5; // 5mins
+    static const TUint kRefreshPeriod       = 1000 * 60 * 60 * 24; // 24hours
+
     friend class SuitePinsManager;
 public:
-    PinsManager(Configuration::IStoreReadWrite& aStore, TUint aMaxDevice);
+    PinsManager(Configuration::IStoreReadWrite& aStore,
+                TUint aMaxDevice,
+                IThreadPool& aThreadPool,
+                ITimerFactory& aTimerFactory,
+                TUint aStartupRefreshDelay = kStartupRefreshDelay,
+                TUint aRefreshPeriod = kRefreshPeriod);
     ~PinsManager();
 public: // from IPinsAccountStore
     void SetAccount(IPinsAccount& aAccount, TUint aCount) override;
 public: // from IPinsInvocable
     void Add(IPinInvoker* aInvoker) override;
+    void Add(IPinMetadataRefresher* aRefresher) override;
 private: // from IPinsManager
     void SetObserver(IPinsObserver& aObserver) override;
     void Set(TUint aIndex, const Brx& aMode, const Brx& aType, const Brx& aUri,
@@ -280,7 +309,11 @@ class PinsManager : public IPinsManager
     void NotifyInvocationCompleted();
     TUint TryParsePinUriVersion(const Brx&) const;
     TBool CheckPinUriHasTokenId(const Brx&) const;
+    void RefreshAll();
+    void RefreshTask();
+    TBool DoRefreshPinsLocked();
 private:
+    const TUint iRefreshPeriod;
     Configuration::IStoreReadWrite& iStore;
     Mutex iLock;
     Mutex iLockInvoke;
@@ -293,8 +326,13 @@ class PinsManager : public IPinsManager
     IPinsAccount* iAccountSetter;
     IPinSetObserver* iPinSetObserver;
     std::map<Brn, IPinInvoker*, BufferCmp> iInvokers;
+    std::map<Brn, IPinMetadataRefresher*, BufferCmp> iRefreshers;
     Pin iInvoke;
+    Pin iUpdated;
     IPinInvoker* iCurrent;
+    IThreadPoolHandle* iRefreshTaskHandle;
+    std::queue<TUint> iRefreshRequests;
+    ITimer* iRefreshTimer;
 };
 
 class AutoPinComplete
@@ -313,7 +351,7 @@ class PinUri
     PinUri(const IPin& aPin);
     ~PinUri();
     const Brx& Mode() const;
-    const Brx& Type() const ;
+    const Brx& Type() const;
     TBool TryGetValue(const TChar* aKey, Brn& aValue) const;
     TBool TryGetValue(const Brx& aKey, Brn& aValue) const;
     TBool TryGetValue(const TChar* aKey, Bwx& aValue) const;

diff --git a/OpenHome/Av/Tests/TestPins.cpp b/OpenHome/Av/Tests/TestPins.cpp
@@ -4,6 +4,7 @@
 #include <OpenHome/Private/Stream.h>
 #include <OpenHome/Configuration/Tests/ConfigRamStore.h>
 #include <OpenHome/Json.h>
+#include <OpenHome/Private/TimerFactoryMock.h>
 
 #include <limits.h>
 #include <vector>
@@ -173,6 +174,8 @@ class SuitePinsManager : public SuiteUnitTest
     void TestInvokeUri();
     void TestInvokeUriInvalidMode();
 private:
+    IThreadPool *iThreadPool;
+    ITimerFactory *iTimerFactory;
     Configuration::ConfigRamStore* iStore;
     PinsManager* iPinsManager;
     TUint iAccountSetIndex;
@@ -738,8 +741,10 @@ SuitePinsManager::SuitePinsManager()
 
 void SuitePinsManager::Setup()
 {
+    iThreadPool = new MockThreadPoolSync();
+    iTimerFactory = new TimerFactoryMock();
     iStore = new Configuration::ConfigRamStore();
-    iPinsManager = new PinsManager(*iStore, kMaxDevicePins);
+    iPinsManager = new PinsManager(*iStore, kMaxDevicePins, *iThreadPool, *iTimerFactory);
 
     // We don't test activity stat callbacks fully.  Registering an observer
     // at least confirms that it doesn't crash in normal use.
@@ -766,6 +771,8 @@ void SuitePinsManager::TearDown()
 {
     delete iPinsManager;
     delete iStore;
+    delete iTimerFactory;
+    delete iThreadPool;
 }
 
 void SuitePinsManager::Set(TUint aIndex, const Brx& aMode, const Brx& aType, const Brx& aUri,