Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DDS device signals, bulk query-option & misc #12517

Merged
merged 17 commits into from
Dec 26, 2023
Merged

DDS device signals, bulk query-option & misc #12517

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
a7873c4
add D555e recovery PID
maloel Dec 7, 2023
166c124
add topic-writer::wait-for-readers()
maloel Dec 12, 2023
93462fc
move device reply status check to write-control-message()
maloel Dec 12, 2023
0397a64
add is_xxx() to rsutils::json::nested
maloel Dec 13, 2023
8986367
add bulk query-option variants
maloel Dec 13, 2023
059cb37
add missing :gha to test:retries, and ensure dds 'enabled'
maloel Dec 13, 2023
b3f6c56
change device on-metadata-available to signal
maloel Dec 13, 2023
d1cf4aa
device debug name in log messages
maloel Dec 14, 2023
f826ac8
integrate options-server into test-options.py
maloel Dec 14, 2023
a06f7c0
change device on-notification to signal
maloel Dec 14, 2023
0764e3b
signal::raise() now returns whether notifications went out
maloel Dec 14, 2023
1671a3e
change device on-device-log to signal
maloel Dec 14, 2023
4e7a18d
add dds_device::check_reply()
maloel Dec 18, 2023
d47b6ae
allow empty string in context configuration
maloel Dec 19, 2023
1caa0b4
revise check_reply
maloel Dec 20, 2023
c3e1069
open participant with NO status events enable
maloel Dec 20, 2023
05b9b43
hopefully clarify the query-option handling in device-impl
maloel Dec 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions src/dds/rs-dds-device-proxy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,10 +496,6 @@ void dds_device_proxy::hardware_reset()
nlohmann::json control = nlohmann::json::object( { { "id", "hw-reset" } } );
nlohmann::json reply;
_dds_dev->send_control( control, &reply );
std::string default_status( "OK", 2 );
if( rsutils::json::get( reply, "status", default_status ) != default_status )
throw std::runtime_error( "Failed to reset: "
+ rsutils::json::get( reply, "explanation", std::string( "unknown reason" ) ) );
}


Expand All @@ -510,10 +506,6 @@ std::vector< uint8_t > dds_device_proxy::send_receive_raw_data( const std::vecto
nlohmann::json control = nlohmann::json::object( { { "id", "hwm" }, { "data", hexdata } } );
nlohmann::json reply;
_dds_dev->send_control( control, &reply );
std::string default_status( "OK", 2 );
if( rsutils::json::get( reply, "status", default_status ) != default_status )
throw std::runtime_error( "Failed HWM: "
+ rsutils::json::get( reply, "explanation", std::string( "unknown reason" ) ) );
rsutils::string::hexarray data;
if( ! rsutils::json::get_ex( reply, "data", &data ) )
throw std::runtime_error( "Failed HWM: missing 'data' in reply" );
Expand Down Expand Up @@ -541,10 +533,6 @@ std::vector< uint8_t > dds_device_proxy::build_command( uint32_t opcode,
{ "build-command", true } } );
nlohmann::json reply;
_dds_dev->send_control( control, &reply );
std::string default_status( "OK", 2 );
if( rsutils::json::get( reply, "status", default_status ) != default_status )
throw std::runtime_error( "Failed build-command: "
+ rsutils::json::get( reply, "explanation", std::string( "unknown reason" ) ) );
if( ! rsutils::json::get_ex( reply, "data", &hexdata ) )
throw std::runtime_error( "Failed HWM: missing 'data' in reply" );
return hexdata.detach();
Expand Down
28 changes: 17 additions & 11 deletions third-party/realdds/src/dds-device-impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,10 +368,6 @@ void dds_device::impl::open( const dds_stream_profiles & profiles )

nlohmann::json reply;
write_control_message( j, &reply );

if( rsutils::json::get( reply, status_key, status_ok ) != status_ok )
throw std::runtime_error( "failed to open stream: "
+ rsutils::json::get< std::string >( reply, explanation_key, "no explanation" ) );
}


Expand All @@ -390,10 +386,6 @@ void dds_device::impl::set_option_value( const std::shared_ptr< dds_option > & o

nlohmann::json reply;
write_control_message( j, &reply );

if( rsutils::json::get( reply, status_key, status_ok ) != status_ok )
throw std::runtime_error(
rsutils::json::get< std::string >( reply, explanation_key, "failed to set-option; no explanation" ) );
//option->set_value( new_value );
}

Expand All @@ -413,9 +405,6 @@ float dds_device::impl::query_option_value( const std::shared_ptr< dds_option >
nlohmann::json reply;
write_control_message( j, &reply );

if( rsutils::json::get( reply, status_key, status_ok ) != status_ok )
throw std::runtime_error(
rsutils::json::get< std::string >( reply, explanation_key, "failed to query-option; no explanation" ) );
return rsutils::json::get< float >( reply, value_key );
}

Expand All @@ -442,6 +431,23 @@ void dds_device::impl::write_control_message( topics::flexible_msg && msg, nlohm
//LOG_DEBUG( "got reply: " << actual_reply );
*reply = std::move( actual_reply );
_replies.erase( this_sequence_number );

if( rsutils::json::get( *reply, status_key, status_ok ) != status_ok )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we brake this to functions

if( reply )
    wait_for_reply( reply );
    check_status( reply );

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks different in the final code:

        *reply = std::move( actual_reply );
        _replies.erase( this_sequence_number );

        // Throw if there's an error
        dds_device::check_reply( *reply );

{
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: ";
if( auto e = rsutils::json::nested( *reply, explanation_key ) )
os << e.get();
else
os << "no explanation";
DDS_THROW( runtime_error, os.str() );
}
}
}

Expand Down