From 1caa0b4af7a8f626fa3f6052ba2ca5031cdd57f1 Mon Sep 17 00:00:00 2001
From: Eran <librealsense.eran@gmail.com>
Date: Wed, 20 Dec 2023 11:24:49 +0200
Subject: [PATCH] revise check_reply

---
 third-party/realdds/src/dds-device-impl.cpp | 20 +++++--------
 third-party/realdds/src/dds-device.cpp      | 31 +++++++++++----------
 unit-tests/dds/test-query-option.py         |  2 +-
 3 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/third-party/realdds/src/dds-device-impl.cpp b/third-party/realdds/src/dds-device-impl.cpp
index 1a3dc7dfdb..5494ce7421 100644
--- a/third-party/realdds/src/dds-device-impl.cpp
+++ b/third-party/realdds/src/dds-device-impl.cpp
@@ -215,6 +215,11 @@ void dds_device::impl::handle_notification( nlohmann::json const & j,
                         replyit->second = std::move( j );
                         _replies_cv.notify_all();
                     }
+                    else
+                    {
+                        // Nobody's waiting for it - but we can still log any errors:
+                        dds_device::check_reply( j );
+                    }
                 }
             }
         }
@@ -456,19 +461,8 @@ void dds_device::impl::write_control_message( topics::flexible_msg && msg, nlohm
         *reply = std::move( actual_reply );
         _replies.erase( this_sequence_number );
 
-        std::string explanation;
-        if( ! dds_device::check_reply( *reply, &explanation ) )
-        {
-            std::ostringstream os;
-            os << "control #" << this_sequence_number;
-            if( auto id = rsutils::json::nested( *reply, control_key, id_key ) )
-            {
-                if( id->is_string() )
-                    os << " \"" << id.string_ref() << "\"";
-            }
-            os << " failed: " << explanation;
-            DDS_THROW( runtime_error, os.str() );
-        }
+        // Throw if there's an error
+        dds_device::check_reply( *reply );
     }
 }
 
diff --git a/third-party/realdds/src/dds-device.cpp b/third-party/realdds/src/dds-device.cpp
index 639cef0923..a67318b8a7 100644
--- a/third-party/realdds/src/dds-device.cpp
+++ b/third-party/realdds/src/dds-device.cpp
@@ -138,6 +138,7 @@ rsutils::subscription dds_device::on_notification( on_notification_callback && c
 static std::string const status_key( "status", 6 );
 static std::string const status_ok( "ok", 2 );
 static std::string const explanation_key( "explanation", 11 );
+static std::string const id_key( "id", 2 );
 
 
 bool dds_device::check_reply( nlohmann::json const & reply, std::string * p_explanation )
@@ -145,31 +146,33 @@ bool dds_device::check_reply( nlohmann::json const & reply, std::string * p_expl
     auto status_j = rsutils::json::nested( reply, status_key );
     if( ! status_j )
         return true;
-    std::string explanation;
+    std::ostringstream os;
     if( ! status_j->is_string() )
-        explanation = rsutils::string::from() << "bad status: " << status_j;
+        os << "bad status " << status_j;
     else if( status_j.string_ref() == status_ok )
         return true;
     else
     {
+        os << "[";
+        if( auto id = rsutils::json::nested( reply, id_key ) )
+        {
+            if( id->is_string() )
+                os << "\"" << id.string_ref() << "\" ";
+        }
+        os << status_j.string_ref() << "]";
         if( auto explanation_j = rsutils::json::nested( reply, explanation_key ) )
         {
+            os << ' ';
             if( ! explanation_j->is_string() || explanation_j.string_ref().empty() )
-                explanation = rsutils::string::from() << "[" << status_j.string_ref() << "] bad explanation: " << explanation_j;
+                os << "bad explanation " << explanation_j;
             else
-                explanation = rsutils::string::from() << "[" << status_j.string_ref() << "] " << explanation_j.string_ref();
+                os << explanation_j.string_ref();
         }
-        else
-            explanation = "no explanation";
-    }
-    if( ! explanation.empty() )
-    {
-        if( ! p_explanation )
-            DDS_THROW( runtime_error, explanation );
-        *p_explanation = std::move( explanation );
-        return false;
     }
-    return true;
+    if( ! p_explanation )
+        DDS_THROW( runtime_error, os.str() );
+    *p_explanation = os.str();
+    return false;
 }
 
 
diff --git a/unit-tests/dds/test-query-option.py b/unit-tests/dds/test-query-option.py
index 7102e32880..616bb5af3f 100644
--- a/unit-tests/dds/test-query-option.py
+++ b/unit-tests/dds/test-query-option.py
@@ -66,7 +66,7 @@
                 'option-name': 'custom option'
             }, True ),  # Wait for reply
             RuntimeError,
-            'control #1 "query-option" failed: [error] \'s1\' option \'custom option\' not found' )
+            '["query-option" error] \'s1\' option \'custom option\' not found' )
 
     with test.closure( 'Query all options, option-name:[]' ):
         reply = device.send_control( {