Unify notification and control messages using "flexible-message" #11063
Conversation
…r everywhere else...
…rrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrr
| { | ||
| } | ||
|
|
||
| int add_dds_stream( std::shared_ptr< realdds::dds_stream > const & stream ) |
There was a problem hiding this comment.
Why int and not size_t? No need to cast
There was a problem hiding this comment.
It's the stream index, which is usually an int in libRS and the profile. We'll need to cast at some point. Might as well be here.
There was a problem hiding this comment.
You cast the sensor index at to_rs2_video_stream, I think this cast should be don't there too
There was a problem hiding this comment.
And this is not exactly how USB device indexes work, still need to work it out. Here you can have depth or color with index other than 0, and infrared with index 3 or 4. OK for this pull request, but needs more thinking
| { | ||
| for( auto & sp : _streams[stream_index]->profiles() ) | ||
| { | ||
| auto vsp = std::static_pointer_cast< realdds::dds_video_stream_profile >( sp ); |
There was a problem hiding this comment.
Need to check if stream type is video
There was a problem hiding this comment.
Right now it assumes the streams are all uniform: they're created from a dds_device which gets its streams from a stream-header that defines the type of its profiles.
|
The previous message size was >10K for D435i. For L515, the color stream had a json that was |
| : _uid( uid ) | ||
| , _format( format ) | ||
| dds_stream_profile( dds_stream_format format, int16_t frequency ) | ||
| : _format( format ) |
There was a problem hiding this comment.
It is more efficient to initialize members by declaration order.
There was a problem hiding this comment.
AFAIK the init order is the order of decl, whatever order you write them in.
| { | ||
| int it = 0; | ||
| auto profile = std::make_shared< final_stream_profile >( j, it ); | ||
| verify_end_of_json( j, it ); // just so it's not in the header |
There was a problem hiding this comment.
Why not put all of from_json implementation in the cpp? It is not inlined anyway
| raw::flexible flexible_msg::to_raw() | ||
| { | ||
| raw::flexible raw_msg; | ||
| raw_msg.data_format( (raw::flexible_data_format) _data_format ); |
| # the uid is not communicated any more... therefore 0x1 (the first profile) | ||
| test.check_equal( '<pyrealdds.motion_stream_profile 0x1 RGB8 30 Hz>', str(profiles[0]) ) | ||
| test.check_equal( len( profiles ), 1 ) | ||
| test.check_equal( str(profiles[0]), '<pyrealdds.motion_stream_profile RGB8 @ 30 Hz>' ) |
There was a problem hiding this comment.
Its just a communication test but RGB8 for motion is wierd :-)
Maybe we need to add a motion format specification?
There was a problem hiding this comment.
There is... but like you said it's not important here.
The next PR will have a whole D435i emulation, with all the profiles including motion.
dds_stream_uid- context-side code now generates them automaticallyoperator<()forrs2::device(otherwise std::map would useoperator bool()and won't complain)