Merge pull request #22366 from emveepee/uneventful-nexus

 [PVR] Connection eventlog changes

9 files changed, 154 insertions, 101 deletions

diff --git a/xbmc/pvr/PVREventLogJob.cpp b/xbmc/pvr/PVREventLogJob.cpp
index 8069c7f3fe..b70ad1cb62 100644
--- a/xbmc/pvr/PVREventLogJob.cpp
+++ b/xbmc/pvr/PVREventLogJob.cpp
@@ -16,14 +16,22 @@
 namespace PVR
 {
 
-CPVREventLogJob::CPVREventLogJob(bool bNotifyUser, bool bError, const std::string& label, const std::string& msg, const std::string& icon)
+CPVREventLogJob::CPVREventLogJob(bool bNotifyUser,
+                                 EventLevel eLevel,
+                                 const std::string& label,
+                                 const std::string& msg,
+                                 const std::string& icon)
 {
-  AddEvent(bNotifyUser, bError, label, msg, icon);
+  AddEvent(bNotifyUser, eLevel, label, msg, icon);
 }
 
-void CPVREventLogJob::AddEvent(bool bNotifyUser, bool bError, const std::string& label, const std::string& msg, const std::string& icon)
+void CPVREventLogJob::AddEvent(bool bNotifyUser,
+                               EventLevel eLevel,
+                               const std::string& label,
+                               const std::string& msg,
+                               const std::string& icon)
 {
-  m_events.emplace_back(Event(bNotifyUser, bError, label, msg, icon));
+  m_events.emplace_back(Event(bNotifyUser, eLevel, label, msg, icon));
 }
 
 bool CPVREventLogJob::DoWork()
@@ -31,15 +39,16 @@ bool CPVREventLogJob::DoWork()
   for (const auto& event : m_events)
   {
     if (event.m_bNotifyUser)
-      CGUIDialogKaiToast::QueueNotification(
-        event.m_bError ? CGUIDialogKaiToast::Error : CGUIDialogKaiToast::Info, event.m_label.c_str(), event.m_msg, 5000, true);
+      CGUIDialogKaiToast::QueueNotification(event.m_eLevel == EventLevel::Error
+                                                ? CGUIDialogKaiToast::Error
+                                                : CGUIDialogKaiToast::Info,
+                                            event.m_label, event.m_msg, 5000, true);
 
     // Write event log entry.
     auto eventLog = CServiceBroker::GetEventLog();
     if (eventLog)
       eventLog->Add(std::make_shared<CNotificationEvent>(event.m_label, event.m_msg, event.m_icon,
-                                                         event.m_bError ? EventLevel::Error
-                                                                        : EventLevel::Information));
+                                                         event.m_eLevel));
   }
   return true;
 }
diff --git a/xbmc/pvr/PVREventLogJob.h b/xbmc/pvr/PVREventLogJob.h
index 62d5751bf0..a5f0be7fa2 100644
--- a/xbmc/pvr/PVREventLogJob.h
+++ b/xbmc/pvr/PVREventLogJob.h
@@ -8,6 +8,7 @@
 
 #pragma once
 
+#include "events/EventLog.h"
 #include "utils/Job.h"
 
 #include <string>
@@ -19,11 +20,21 @@ class CPVREventLogJob : public CJob
 {
 public:
   CPVREventLogJob() = default;
-  CPVREventLogJob(bool bNotifyUser, bool bError, const std::string& label, const std::string& msg, const std::string& icon);
+
+  CPVREventLogJob(bool bNotifyUser,
+                  EventLevel eLevel,
+                  const std::string& label,
+                  const std::string& msg,
+                  const std::string& icon);
+
   ~CPVREventLogJob() override = default;
   const char* GetType() const override { return "pvr-eventlog-job"; }
 
-  void AddEvent(bool bNotifyUser, bool bError, const std::string& label, const std::string& msg, const std::string& icon);
+  void AddEvent(bool bNotifyUser,
+                EventLevel eLevel,
+                const std::string& label,
+                const std::string& msg,
+                const std::string& icon);
 
   bool DoWork() override;
 
@@ -31,13 +42,19 @@ private:
   struct Event
   {
     bool m_bNotifyUser;
-    bool m_bError;
+    EventLevel m_eLevel{EventLevel::Information};
     std::string m_label;
     std::string m_msg;
     std::string m_icon;
 
-    Event(bool bNotifyUser, bool bError, const std::string& label, const std::string& msg, const std::string& icon)
-    : m_bNotifyUser(bNotifyUser), m_bError(bError), m_label(label), m_msg(msg), m_icon(icon) {}
+    Event(bool bNotifyUser,
+          EventLevel elevel,
+          const std::string& label,
+          const std::string& msg,
+          const std::string& icon)
+      : m_bNotifyUser(bNotifyUser), m_eLevel(elevel), m_label(label), m_msg(msg), m_icon(icon)
+    {
+    }
   };
 
   std::vector<Event> m_events;
diff --git a/xbmc/pvr/addons/PVRClient.cpp b/xbmc/pvr/addons/PVRClient.cpp
index c6b2206e25..f6787cb532 100644
--- a/xbmc/pvr/addons/PVRClient.cpp
+++ b/xbmc/pvr/addons/PVRClient.cpp
@@ -125,7 +125,6 @@ void CPVRClient::ResetProperties()
   m_bPriorityFetched = false;
   m_strBackendVersion = DEFAULT_INFO_STRING_VALUE;
   m_strConnectionString = DEFAULT_INFO_STRING_VALUE;
-  m_strFriendlyName = DEFAULT_INFO_STRING_VALUE;
   m_strBackendName = DEFAULT_INFO_STRING_VALUE;
   m_strBackendHostname.clear();
   m_menuhooks.reset();
@@ -243,6 +242,11 @@ void CPVRClient::SetConnectionState(PVR_CONNECTION_STATE state)
     if (!GetAddonProperties())
       CLog::LogF(LOGERROR, "Error reading PVR client properties");
   }
+  else
+  {
+    if (!GetAddonNameStringProperties())
+      CLog::LogF(LOGERROR, "Cannot read PVR client name string properties");
+  }
 
   std::unique_lock<CCriticalSection> lock(m_critSection);
 
@@ -290,11 +294,9 @@ int CPVRClient::GetID() const
 
 bool CPVRClient::GetAddonProperties()
 {
-  char strBackendName[PVR_ADDON_NAME_STRING_LENGTH] = {};
-  char strConnectionString[PVR_ADDON_NAME_STRING_LENGTH] = {};
-  char strBackendVersion[PVR_ADDON_NAME_STRING_LENGTH] = {};
-  char strBackendHostname[PVR_ADDON_NAME_STRING_LENGTH] = {};
-  std::string strFriendlyName;
+  if (!GetAddonNameStringProperties())
+    return false;
+
   PVR_ADDON_CAPABILITIES addonCapabilities = {};
   std::vector<std::shared_ptr<CPVRTimerType>> timerTypes;
 
@@ -309,62 +311,10 @@ bool CPVRClient::GetAddonProperties()
   if (retVal != PVR_ERROR_NO_ERROR)
     return false;
 
-  /* get the name of the backend */
-  retVal = DoAddonCall(
-      __func__,
-      [&strBackendName](const AddonInstance* addon) {
-        return addon->toAddon->GetBackendName(addon, strBackendName, sizeof(strBackendName));
-      },
-      true, false);
-
-  if (retVal != PVR_ERROR_NO_ERROR)
-    return false;
-
-  /* get the connection string */
-  retVal = DoAddonCall(
-      __func__,
-      [&strConnectionString](const AddonInstance* addon) {
-        return addon->toAddon->GetConnectionString(addon, strConnectionString,
-                                                   sizeof(strConnectionString));
-      },
-      true, false);
-
-  if (retVal != PVR_ERROR_NO_ERROR && retVal != PVR_ERROR_NOT_IMPLEMENTED)
-    return false;
-
-  /* display name = backend name:connection string */
-  strFriendlyName = strlen(strConnectionString) == 0 // connection string is optional
-                        ? strBackendName
-                        : StringUtils::Format("{}:{}", strBackendName, strConnectionString);
-
-  /* backend version number */
-  retVal = DoAddonCall(
-      __func__,
-      [&strBackendVersion](const AddonInstance* addon) {
-        return addon->toAddon->GetBackendVersion(addon, strBackendVersion,
-                                                 sizeof(strBackendVersion));
-      },
-      true, false);
-
-  if (retVal != PVR_ERROR_NO_ERROR)
-    return false;
-
-  /* backend hostname */
-  retVal = DoAddonCall(
-      __func__,
-      [&strBackendHostname](const AddonInstance* addon) {
-        return addon->toAddon->GetBackendHostname(addon, strBackendHostname,
-                                                  sizeof(strBackendHostname));
-      },
-      true, false);
-
-  if (retVal != PVR_ERROR_NO_ERROR && retVal != PVR_ERROR_NOT_IMPLEMENTED)
-    return false;
-
   /* timer types */
   retVal = DoAddonCall(
       __func__,
-      [this, strFriendlyName, &addonCapabilities, &timerTypes](const AddonInstance* addon) {
+      [this, &addonCapabilities, &timerTypes](const AddonInstance* addon) {
         std::unique_ptr<PVR_TIMER_TYPE[]> types_array(
             new PVR_TIMER_TYPE[PVR_ADDON_TIMERTYPE_ARRAY_SIZE]);
         int size = PVR_ADDON_TIMERTYPE_ARRAY_SIZE;
@@ -377,7 +327,7 @@ bool CPVRClient::GetAddonProperties()
           CLog::LogF(LOGWARNING,
                      "Add-on {} does not support timer types. It will work, but not benefit from "
                      "the timer features introduced with PVR Addon API 2.0.0",
-                     strFriendlyName);
+                     GetFriendlyName());
 
           // Create standard timer types (mostly) matching the timer functionality available in Isengard.
           // This is for migration only and does not make changes to the addons obsolete. Addons should
@@ -449,15 +399,74 @@ bool CPVRClient::GetAddonProperties()
 
   /* update the members */
   std::unique_lock<CCriticalSection> lock(m_critSection);
+  m_clientCapabilities = addonCapabilities;
+  m_timertypes = timerTypes;
+
+  return retVal == PVR_ERROR_NO_ERROR;
+}
+
+bool CPVRClient::GetAddonNameStringProperties()
+{
+  char strBackendName[PVR_ADDON_NAME_STRING_LENGTH] = {};
+  char strConnectionString[PVR_ADDON_NAME_STRING_LENGTH] = {};
+  char strBackendVersion[PVR_ADDON_NAME_STRING_LENGTH] = {};
+  char strBackendHostname[PVR_ADDON_NAME_STRING_LENGTH] = {};
+
+  /* get the name of the backend */
+  PVR_ERROR retVal = DoAddonCall(
+      __func__,
+      [&strBackendName](const AddonInstance* addon) {
+        return addon->toAddon->GetBackendName(addon, strBackendName, sizeof(strBackendName));
+      },
+      true, false);
+
+  if (retVal != PVR_ERROR_NO_ERROR)
+    return false;
+
+  /* get the connection string */
+  retVal = DoAddonCall(
+      __func__,
+      [&strConnectionString](const AddonInstance* addon) {
+        return addon->toAddon->GetConnectionString(addon, strConnectionString,
+                                                   sizeof(strConnectionString));
+      },
+      true, false);
+
+  if (retVal != PVR_ERROR_NO_ERROR && retVal != PVR_ERROR_NOT_IMPLEMENTED)
+    return false;
+
+  /* backend version number */
+  retVal = DoAddonCall(
+      __func__,
+      [&strBackendVersion](const AddonInstance* addon) {
+        return addon->toAddon->GetBackendVersion(addon, strBackendVersion,
+                                                 sizeof(strBackendVersion));
+      },
+      true, false);
+
+  if (retVal != PVR_ERROR_NO_ERROR)
+    return false;
+
+  /* backend hostname */
+  retVal = DoAddonCall(
+      __func__,
+      [&strBackendHostname](const AddonInstance* addon) {
+        return addon->toAddon->GetBackendHostname(addon, strBackendHostname,
+                                                  sizeof(strBackendHostname));
+      },
+      true, false);
+
+  if (retVal != PVR_ERROR_NO_ERROR && retVal != PVR_ERROR_NOT_IMPLEMENTED)
+    return false;
+
+  /* update the members */
+  std::unique_lock<CCriticalSection> lock(m_critSection);
   m_strBackendName = strBackendName;
   m_strConnectionString = strConnectionString;
-  m_strFriendlyName = strFriendlyName;
   m_strBackendVersion = strBackendVersion;
-  m_clientCapabilities = addonCapabilities;
   m_strBackendHostname = strBackendHostname;
-  m_timertypes = timerTypes;
 
-  return retVal == PVR_ERROR_NO_ERROR;
+  return true;
 }
 
 const std::string& CPVRClient::GetBackendName() const
@@ -480,9 +489,17 @@ const std::string& CPVRClient::GetConnectionString() const
   return m_strConnectionString;
 }
 
-const std::string& CPVRClient::GetFriendlyName() const
+const std::string CPVRClient::GetFriendlyName() const
 {
-  return m_strFriendlyName;
+
+  if (Addon()->SupportsInstanceSettings())
+  {
+    std::string instanceName;
+    Addon()->GetSettingString(ADDON_SETTING_INSTANCE_NAME_VALUE, instanceName, InstanceId());
+    if (!instanceName.empty())
+      return StringUtils::Format("{} ({})", Name(), instanceName);
+  }
+  return Name();
 }
 
 PVR_ERROR CPVRClient::GetDriveSpace(uint64_t& iTotal, uint64_t& iUsed)
@@ -1788,8 +1805,8 @@ void CPVRClient::cb_recording_notification(void* kodiInstance,
       return;
     }
 
-    const std::string strLine1 =
-        StringUtils::Format(g_localizeStrings.Get(bOnOff ? 19197 : 19198), client->Name());
+    const std::string strLine1 = StringUtils::Format(g_localizeStrings.Get(bOnOff ? 19197 : 19198),
+                                                     client->GetFriendlyName());
     std::string strLine2;
     if (strName)
       strLine2 = strName;
@@ -1801,8 +1818,8 @@ void CPVRClient::cb_recording_notification(void* kodiInstance,
                                           false);
     auto eventLog = CServiceBroker::GetEventLog();
     if (eventLog)
-      eventLog->Add(
-          EventPtr(new CNotificationEvent(client->Name(), strLine1, client->Icon(), strLine2)));
+      eventLog->Add(EventPtr(
+          new CNotificationEvent(client->GetFriendlyName(), strLine1, client->Icon(), strLine2)));
 
     CLog::LogFC(LOGDEBUG, LOGPVR, "Recording {} on client '{}'. name='{}' filename='{}'",
                 bOnOff ? "started" : "finished", client->ID(), strName, strFileName);
diff --git a/xbmc/pvr/addons/PVRClient.h b/xbmc/pvr/addons/PVRClient.h
index e007158bb7..10203cf515 100644
--- a/xbmc/pvr/addons/PVRClient.h
+++ b/xbmc/pvr/addons/PVRClient.h
@@ -163,9 +163,10 @@ public:
   const std::string& GetConnectionString() const;
 
   /*!
-   * @return A friendly name for this add-on that can be used in log messages.
+   * @brief A friendly name used to uniquely identify the addon instance
+   * @return string that can be used in log messages and the GUI.
    */
-  const std::string& GetFriendlyName() const;
+  const std::string GetFriendlyName() const;
 
   /*!
    * @brief Get the disk space reported by the server.
@@ -801,6 +802,12 @@ private:
   bool GetAddonProperties();
 
   /*!
+   * @brief reads the client's name string properties
+   * @return True on success, false otherwise.
+   */
+  bool GetAddonNameStringProperties();
+
+  /*!
    * @brief Write the given addon properties to the given properties container.
    * @param properties Pointer to an array of addon properties.
    * @param iPropertyCount The number of properties contained in the addon properties array.
@@ -1042,7 +1049,6 @@ private:
   std::string m_strBackendName; /*!< the cached backend version */
   std::string m_strBackendVersion; /*!< the cached backend version */
   std::string m_strConnectionString; /*!< the cached connection string */
-  std::string m_strFriendlyName; /*!< the cached friendly name */
   std::string m_strBackendHostname; /*!< the cached backend hostname */
   CPVRClientCapabilities m_clientCapabilities; /*!< the cached add-on's capabilities */
   std::shared_ptr<CPVRClientMenuHooks> m_menuhooks; /*!< the menu hooks for this add-on */
diff --git a/xbmc/pvr/addons/PVRClients.cpp b/xbmc/pvr/addons/PVRClients.cpp
index 4ee6c6f516..42daa85f62 100644
--- a/xbmc/pvr/addons/PVRClients.cpp
+++ b/xbmc/pvr/addons/PVRClients.cpp
@@ -23,6 +23,7 @@
 #include "pvr/channels/PVRChannelGroupInternal.h"
 #include "pvr/guilib/PVRGUIProgressHandler.h"
 #include "utils/JobManager.h"
+#include "utils/StringUtils.h"
 #include "utils/log.h"
 
 #include <algorithm>
@@ -170,10 +171,10 @@ void CPVRClients::UpdateClients(
         {
           CServiceBroker::GetAddonMgr().DisableAddon(client->ID(),
                                                      AddonDisabledReason::PERMANENT_FAILURE);
-          CServiceBroker::GetJobManager()->AddJob(new CPVREventLogJob(true, true, client->Name(),
-                                                                      g_localizeStrings.Get(24070),
-                                                                      client->Icon()),
-                                                  nullptr);
+          CServiceBroker::GetJobManager()->AddJob(
+              new CPVREventLogJob(true, EventLevel::Error, client->Name(),
+                                  g_localizeStrings.Get(24070), client->Icon()),
+              nullptr);
         }
       }
     }
@@ -803,7 +804,7 @@ void CPVRClients::ConnectionStateChange(CPVRClient* client,
     return;
 
   int iMsg = -1;
-  bool bError = true;
+  EventLevel eLevel = EventLevel::Error;
   bool bNotify = true;
 
   switch (newState)
@@ -828,7 +829,7 @@ void CPVRClients::ConnectionStateChange(CPVRClient* client,
       iMsg = 35508; // Access denied
       break;
     case PVR_CONNECTION_STATE_CONNECTED:
-      bError = false;
+      eLevel = EventLevel::Basic;
       iMsg = 36034; // Connection established
       if (client->GetPreviousConnectionState() == PVR_CONNECTION_STATE_UNKNOWN ||
           client->GetPreviousConnectionState() == PVR_CONNECTION_STATE_CONNECTING)
@@ -838,7 +839,7 @@ void CPVRClients::ConnectionStateChange(CPVRClient* client,
       iMsg = 36030; // Connection lost
       break;
     case PVR_CONNECTION_STATE_CONNECTING:
-      bError = false;
+      eLevel = EventLevel::Information;
       iMsg = 35509; // Connecting
       bNotify = false;
       break;
@@ -854,9 +855,12 @@ void CPVRClients::ConnectionStateChange(CPVRClient* client,
   else
     strMsg = g_localizeStrings.Get(iMsg);
 
+  if (!strConnectionString.empty())
+    strMsg = StringUtils::Format("{} ({})", strMsg, strConnectionString);
+
   // Notify user.
   CServiceBroker::GetJobManager()->AddJob(
-      new CPVREventLogJob(bNotify, bError, client->GetFriendlyName(), strMsg, client->Icon()),
+      new CPVREventLogJob(bNotify, eLevel, client->GetFriendlyName(), strMsg, client->Icon()),
       nullptr);
 }
 
diff --git a/xbmc/pvr/addons/PVRClients.h b/xbmc/pvr/addons/PVRClients.h
index a63638f6c8..32248fa322 100644
--- a/xbmc/pvr/addons/PVRClients.h
+++ b/xbmc/pvr/addons/PVRClients.h
@@ -397,9 +397,9 @@ struct SBackend
     /*!
      * @brief Notify a change of an addon connection state.
      * @param client The changed client.
-     * @param strConnectionString A human-readable string identifiying the addon.
+     * @param strConnectionString A human-readable string providing additional information.
      * @param newState The new connection state.
-     * @param strMessage A human readable message providing additional information.
+     * @param strMessage A human readable string replacing default state message.
      */
     void ConnectionStateChange(CPVRClient* client,
                                const std::string& strConnectionString,
diff --git a/xbmc/pvr/guilib/PVRGUIActionsClients.cpp b/xbmc/pvr/guilib/PVRGUIActionsClients.cpp
index 9696d9392e..b9c598c794 100644
--- a/xbmc/pvr/guilib/PVRGUIActionsClients.cpp
+++ b/xbmc/pvr/guilib/PVRGUIActionsClients.cpp
@@ -73,7 +73,7 @@ bool CPVRGUIActionsClients::ProcessSettingsMenuHooks()
       if (clients.size() == 1)
         pDialog->Add(hook.second.GetLabel());
       else
-        pDialog->Add(hook.first->GetBackendName() + ": " + hook.second.GetLabel());
+        pDialog->Add(hook.first->GetFriendlyName() + ": " + hook.second.GetLabel());
     }
 
     pDialog->Open();
diff --git a/xbmc/pvr/guilib/PVRGUIActionsTimers.cpp b/xbmc/pvr/guilib/PVRGUIActionsTimers.cpp
index d9a0fa1dc5..be8e313aa6 100644
--- a/xbmc/pvr/guilib/PVRGUIActionsTimers.cpp
+++ b/xbmc/pvr/guilib/PVRGUIActionsTimers.cpp
@@ -829,7 +829,7 @@ void AddEventLogEntry(const std::shared_ptr<CPVRTimerInfoTag>& timer, int idEpg,
       CServiceBroker::GetPVRManager().GetClient(timer->GetTimerType()->GetClientId());
   if (client)
   {
-    name = client->Name();
+    name = client->GetFriendlyName();
     icon = client->Icon();
   }
   else
@@ -840,7 +840,7 @@ void AddEventLogEntry(const std::shared_ptr<CPVRTimerInfoTag>& timer, int idEpg,
 
   CPVREventLogJob* job = new CPVREventLogJob;
   job->AddEvent(false, // do not display a toast, only log event
-                false, // info, no error
+                EventLevel::Information, // info, no error
                 name, GetAnnouncerText(timer, idEpg, idNoEpg), icon);
   CServiceBroker::GetJobManager()->AddJob(job, nullptr);
 }
diff --git a/xbmc/pvr/timers/PVRTimers.cpp b/xbmc/pvr/timers/PVRTimers.cpp
index e45fc341b6..760bb192f7 100644
--- a/xbmc/pvr/timers/PVRTimers.cpp
+++ b/xbmc/pvr/timers/PVRTimers.cpp
@@ -387,8 +387,8 @@ bool CPVRTimers::UpdateEntries(const CPVRTimersContainer& timers,
         if (client)
         {
           job->AddEvent(m_settings.GetBoolValue(CSettings::SETTING_PVRRECORD_TIMERNOTIFICATIONS),
-                        false, // info, no error
-                        client->Name(), entry.second, client->Icon());
+                        EventLevel::Information, // info, no error
+                        client->GetFriendlyName(), entry.second, client->Icon());
         }
       }