revise check_reply

maloel · Dec 20, 2023 · 1caa0b4 · 1caa0b4
1 parent d47b6ae
commit 1caa0b4
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 28 deletions.
diff --git 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
@@ -138,38 +138,41 @@ 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 )
 {
     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
@@ -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( {