MediaClusters: Update SkipChannel per issue 26104 (#26501)

* MediaClusters: Update SkipChannel per issue 26104 * Update delegate methods * Apply restyle fix * Update cast issue * Update the generated files * Update logic & tests * Apply restyle * Updated code again * Updated tests * Adding random comments just to retrigger restyle * Restyle fix * Update tests * Fix conflicts * Update tests & code * Restyle fix * Update Test due to previous index calculation error * Add fix for tests * Update test & suggestion by Boris * Fix the issue with max value of channels * Simplify the decrease channel formula * Simplify the decrease channel formula * Rebase to the latest code + add Boris suggestion * Simplify code per Boris suggestion
project-chip · Aug 1, 2023 · 6b3f68c · 6b3f68c
1 parent 946cfee
commit 6b3f68c
Show file tree

Hide file tree

Showing 19 changed files with 3,321 additions and 64 deletions.
diff --git a/examples/all-clusters-minimal-app/all-clusters-common/all-clusters-minimal-app.matter b/examples/all-clusters-minimal-app/all-clusters-common/all-clusters-minimal-app.matter
@@ -3083,7 +3083,7 @@ server cluster Channel = 1284 {
   }
 
   request struct SkipChannelRequest {
-    INT16U count = 0;
+    INT16S count = 0;
   }
 
   command ChangeChannelByNumber(ChangeChannelByNumberRequest): DefaultSuccess = 2;

diff --git a/examples/tv-app/android/java/ChannelManager.cpp b/examples/tv-app/android/java/ChannelManager.cpp
@@ -330,7 +330,7 @@ bool ChannelManager::HandleChangeChannelByNumber(const uint16_t & majorNumber, c
     return static_cast<bool>(ret);
 }
 
-bool ChannelManager::HandleSkipChannel(const uint16_t & count)
+bool ChannelManager::HandleSkipChannel(const int16_t & count)
 {
     jboolean ret = JNI_FALSE;
     JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();

diff --git a/examples/tv-app/android/java/ChannelManager.h b/examples/tv-app/android/java/ChannelManager.h
@@ -38,7 +38,7 @@ class ChannelManager : public ChannelDelegate
 
     void HandleChangeChannel(CommandResponseHelper<ChangeChannelResponseType> & helper, const CharSpan & match) override;
     bool HandleChangeChannelByNumber(const uint16_t & majorNumber, const uint16_t & minorNumber) override;
-    bool HandleSkipChannel(const uint16_t & count) override;
+    bool HandleSkipChannel(const int16_t & count) override;
 
     uint32_t GetFeatureMap(chip::EndpointId endpoint) override;
 

diff --git a/examples/tv-app/linux/include/channel/ChannelManager.cpp b/examples/tv-app/linux/include/channel/ChannelManager.cpp
@@ -111,13 +111,19 @@ void ChannelManager::HandleChangeChannel(CommandResponseHelper<ChangeChannelResp
     uint16_t index = 0;
     for (auto const & channel : mChannels)
     {
-        index++;
         // verify if CharSpan matches channel name
         // or callSign or affiliateCallSign or majorNumber.minorNumber
         if (isChannelMatched(channel, match))
         {
             matchedChannels.push_back(channel);
         }
+        else if (matchedChannels.size() == 0)
+        {
+            // "index" is only used when we end up with matchedChannels.size() == 1.
+            // In that case, we want it to be the number of non-matching channels we saw before
+            // the matching one.
+            index++;
+        }
     }
 
     ChangeChannelResponseType response;
@@ -151,7 +157,6 @@ bool ChannelManager::HandleChangeChannelByNumber(const uint16_t & majorNumber, c
     uint16_t index      = 0;
     for (auto const & channel : mChannels)
     {
-        index++;
         // verify if major & minor matches one of the channel from the list
         if (channel.minorNumber == minorNumber && channel.majorNumber == majorNumber)
         {
@@ -163,16 +168,26 @@ bool ChannelManager::HandleChangeChannelByNumber(const uint16_t & majorNumber, c
                 mCurrentChannel      = channel;
             }
         }
+        index++;
     }
     return channelChanged;
 }
 
-bool ChannelManager::HandleSkipChannel(const uint16_t & count)
+bool ChannelManager::HandleSkipChannel(const int16_t & count)
 {
-    // TODO: Insert code here
-    uint16_t newChannelIndex = static_cast<uint16_t>((count + mCurrentChannelIndex) % mChannels.size());
-    mCurrentChannelIndex     = newChannelIndex;
-    mCurrentChannel          = mChannels[mCurrentChannelIndex];
+    int32_t newChannelIndex = static_cast<int32_t>(count) + static_cast<int32_t>(mCurrentChannelIndex);
+    uint16_t channelsSize   = static_cast<uint16_t>(mChannels.size());
+
+    // handle newChannelIndex out of range.
+    newChannelIndex = newChannelIndex % channelsSize;
+
+    if (newChannelIndex < 0)
+    {
+        newChannelIndex = newChannelIndex + channelsSize;
+    }
+
+    mCurrentChannelIndex = static_cast<uint16_t>(newChannelIndex);
+    mCurrentChannel      = mChannels[mCurrentChannelIndex];
     return true;
 }
 

diff --git a/examples/tv-app/linux/include/channel/ChannelManager.h b/examples/tv-app/linux/include/channel/ChannelManager.h
@@ -39,7 +39,7 @@ class ChannelManager : public ChannelDelegate
 
     void HandleChangeChannel(CommandResponseHelper<ChangeChannelResponseType> & helper, const CharSpan & match) override;
     bool HandleChangeChannelByNumber(const uint16_t & majorNumber, const uint16_t & minorNumber) override;
-    bool HandleSkipChannel(const uint16_t & count) override;
+    bool HandleSkipChannel(const int16_t & count) override;
 
     uint32_t GetFeatureMap(chip::EndpointId endpoint) override;
 

diff --git a/examples/tv-app/tv-common/tv-app.matter b/examples/tv-app/tv-common/tv-app.matter
@@ -1850,7 +1850,7 @@ server cluster Channel = 1284 {
   }
 
   request struct SkipChannelRequest {
-    INT16U count = 0;
+    INT16S count = 0;
   }
 
   response struct ChangeChannelResponse = 1 {

diff --git a/examples/tv-casting-app/tv-casting-common/tv-casting-app.matter b/examples/tv-casting-app/tv-casting-common/tv-casting-app.matter
@@ -1370,7 +1370,7 @@ client cluster Channel = 1284 {
   }
 
   request struct SkipChannelRequest {
-    INT16U count = 0;
+    INT16S count = 0;
   }
 
   /** Change the channel on the media player to the channel case-insensitive exact matching the value passed as an argument. */

diff --git a/src/app/clusters/channel-server/channel-delegate.h b/src/app/clusters/channel-server/channel-delegate.h
@@ -42,7 +42,7 @@ class Delegate
     virtual void HandleChangeChannel(CommandResponseHelper<Commands::ChangeChannelResponse::Type> & helper,
                                      const chip::CharSpan & match)                                       = 0;
     virtual bool HandleChangeChannelByNumber(const uint16_t & majorNumber, const uint16_t & minorNumber) = 0;
-    virtual bool HandleSkipChannel(const uint16_t & count)                                               = 0;
+    virtual bool HandleSkipChannel(const int16_t & count)                                                = 0;
 
     bool HasFeature(chip::EndpointId endpoint, ChannelFeature feature);
     virtual uint32_t GetFeatureMap(chip::EndpointId endpoint) = 0;