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.

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

Open streams working for all stream types + some bug fixes #11045

Merged
merged 25 commits into from
Nov 15, 2022
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
f64defa
Create dds_subscriber and dds_control_server
OhadMeir Oct 30, 2022
e23f732
Add UYVY support to dds_stream_profile
OhadMeir Oct 30, 2022
634aa7b
Fix control topic type
OhadMeir Oct 30, 2022
471e6f2
Add dynamic type support for dds-topic
OhadMeir Oct 30, 2022
63c9544
Fix rs-dds-sniffer reader history mode
OhadMeir Oct 30, 2022
cf0e141
Fix dds-device-server motion profile type
OhadMeir Oct 30, 2022
cab9905
Add type information to dds_stream_profile
OhadMeir Oct 30, 2022
a655f4d
Fix bug with group name at dds_device_server
OhadMeir Oct 30, 2022
7d6185e
Type in stream not profile and other PR#11045 comments
OhadMeir Oct 31, 2022
6493c65
Roll back stream type changes (will get from Eran's PR)
OhadMeir Nov 6, 2022
ac6f8bb
Merge branch 'dds' of https://github.com/IntelRealSense/librealsense …
OhadMeir Nov 6, 2022
cf39725
rs-dds-server open streams logic (not tested)
OhadMeir Nov 7, 2022
f7dbb75
Use dds-topic-reader not dds-control-server
OhadMeir Nov 8, 2022
189ea50
rs-dds-server streams upon request (not automatically)
OhadMeir Nov 8, 2022
9890c59
Merge branch 'dds' of https://github.com/IntelRealSense/librealsense …
OhadMeir Nov 8, 2022
afe730e
support open all stream types
OhadMeir Nov 9, 2022
4f9f8a5
rs-dds-server open all kinds of streams
OhadMeir Nov 9, 2022
4480f74
Fixed bug in rs-dds-server - second participant opened
OhadMeir Nov 10, 2022
a7abce6
Using dispathcer to handle controls
OhadMeir Nov 10, 2022
cc05f8c
Fix for linux compilation
OhadMeir Nov 10, 2022
a2f5fe2
Implement close-streams
OhadMeir Nov 10, 2022
f57ffa1
lrs-device-controller holds device streams
OhadMeir Nov 11, 2022
09443aa
Handle PR#11045 comments
OhadMeir Nov 13, 2022
8b54053
Fix notifications writer history size
OhadMeir Nov 14, 2022
3985402
Update dds-notification-server.cpp
maloel Nov 15, 2022
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
4 changes: 2 additions & 2 deletions src/context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ namespace librealsense
rs2_video_stream to_rs2_video_stream( std::shared_ptr< realdds::dds_video_stream_profile > const & profile )
{
rs2_video_stream prof;
prof.type = RS2_STREAM_ANY;
prof.type = static_cast< rs2_stream >(profile->type());
prof.index = profile->uid().index;
prof.uid = profile->uid().sid;
prof.width = profile->width();
Expand All @@ -519,7 +519,7 @@ namespace librealsense
rs2_motion_stream to_rs2_motion_stream( std::shared_ptr< realdds::dds_motion_stream_profile > const & profile )
{
rs2_motion_stream prof;
prof.type = RS2_STREAM_ANY;
prof.type = static_cast< rs2_stream >( profile->type() );
prof.index = profile->uid().index;
prof.uid = profile->uid().sid;
prof.fps = profile->frequency();
Expand Down
47 changes: 47 additions & 0 deletions third-party/realdds/include/realdds/dds-control-server.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// License: Apache 2.0. See LICENSE file in root directory.
// Copyright(c) 2022 Intel Corporation. All Rights Reserved.

#pragma once


#include <realdds/topics/control/controlPubSubTypes.h> // raw::device::control
#include <librealsense2/utilities/concurrency/concurrency.h>

#include <memory>
#include <string>


namespace realdds {


namespace topics {
namespace raw {
namespace device {
class control;
}
} // namespace raw
} // namespace topics


class dds_subscriber;
class dds_topic_reader;


// A control is a command sent from a client (camera user) and the server (this class) receives and interprets.
//
class dds_control_server
Copy link
Collaborator

Choose a reason for hiding this comment

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

From my understanding of the usage, this is not really a server but a client -- a listener for control messages.

If so -- then how is this different than a topic-reader? Do you envision more logic being placed here? I don't see much logic belonging in realdds other than high-level commands (stream start/stop, control get/set) that would still need customization by the end implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that we will need more logic to handle stuff like query-reply pairs and msg-id matching.
If it will all be in the application than you are right and we can refactor this out

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it'll be where the control messages are handled.
Most controls will be handled in the app and only a few will be done entirely in realdds and even then it'll likely be in the device-server. If there's a lot of code we can certainly push to another class but at this time I don't see much justification.

{
public:
dds_control_server( std::shared_ptr< dds_subscriber > const & subscriber, const std::string & topic_name );
~dds_control_server();

typedef std::function< void() > on_control_message_received_callback;
void on_control_message_received( on_control_message_received_callback callback );

private:
std::shared_ptr< dds_subscriber > _subscriber;
std::shared_ptr< dds_topic_reader > _reader;
};


}
7 changes: 7 additions & 0 deletions third-party/realdds/include/realdds/dds-device-server.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ namespace raw {
class device_info;
namespace device {
class notification;
class control;
} // namespace device
} // namespace raw
class device_info;
Expand All @@ -26,8 +27,10 @@ class device_info;

class dds_participant;
class dds_publisher;
class dds_subscriber;
class dds_stream_server;
class dds_notification_server;
class dds_control_server;
struct image_header;


Expand Down Expand Up @@ -65,10 +68,14 @@ class dds_device_server
void publish_notification( topics::raw::device::notification&& notification );

private:
void on_control_message_received();

std::shared_ptr< dds_publisher > _publisher;
std::shared_ptr< dds_subscriber > _subscriber;
std::string _topic_root;
std::unordered_map<std::string, std::shared_ptr<dds_stream_server>> _stream_name_to_server;
std::shared_ptr< dds_notification_server > _notification_server;
std::shared_ptr< dds_control_server > _control_server;
}; // class dds_device_server


Expand Down
16 changes: 10 additions & 6 deletions third-party/realdds/include/realdds/dds-stream-profile.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,18 +83,20 @@ class dds_stream_profile
{
dds_stream_uid _uid;
dds_stream_format _format;
int16_t _frequency; // "Frames" per second
int16_t _frequency; // "Frames" per second
int8_t _inner_type; // User defined type
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't quite understand the need here.
If really necessary, the "type" is the "stream type" and not the profile type. All profiles of the same stream will have the same type, no? I.e., it should be communicated with the stream and not the profile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

RS applications like rs-enumerate-devices and rs-sensor-control use rs2_profile type field to print a "stream name" for each profile, and we need to pass this information to them.

I agree that this is a duplication that can be avoided by sending a stream type. Currently in notificaiton-msg it is sent in the profile struct, and I didn't want to change that so we won't have a difficult merge.

I will take the information to the stream from the first profile in the message assuming all profiles have the same type (true for libRS, not sure that true in the general case)

Copy link
Collaborator

Choose a reason for hiding this comment

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

So approve my PR and sending a type as part of the stream-header will be simple :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

By the way -- I don't think the type should be an argument to the stream, but a subtype of the stream: i.e., a dds_gyro_stream_server derives from dds_motion_stream_server, etc...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then we will have to add types to realdds if LibRS adds a type. realdds doesn't really care about the type, it just need to relay it to the user

Copy link
Collaborator

Choose a reason for hiding this comment

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

But the type does change things, and can change functionality.
Right now it's divided into "video" and "motion" but if we need more, then more types are needed.
Doing this thru polymorphism is the right way to do it -- if it's important. I'm still unsure that it is (what depends on the stream type?) -- let's talk about it.


std::weak_ptr< dds_stream_base > _stream;

public:
virtual ~dds_stream_profile() {}

protected:
dds_stream_profile( dds_stream_uid uid, dds_stream_format format, int16_t frequency )
dds_stream_profile( dds_stream_uid uid, dds_stream_format format, int16_t frequency, int8_t inner_type = 0 )
: _uid( uid )
, _format( format )
, _frequency( frequency )
, _inner_type( inner_type )
{
}
dds_stream_profile( dds_stream_profile && ) = default;
Expand All @@ -107,6 +109,7 @@ class dds_stream_profile
dds_stream_uid uid() const { return _uid; }
dds_stream_format format() const { return _format; }
int16_t frequency() const { return _frequency; }
int8_t type() const { return _inner_type; }

// These are for debugging - not functional
virtual std::string to_string() const;
Expand All @@ -125,8 +128,9 @@ class dds_video_stream_profile : public dds_stream_profile
uint8_t _bytes_per_pixel;

public:
dds_video_stream_profile( dds_stream_uid uid, dds_stream_format format, int16_t frequency, uint16_t width, uint16_t height, uint8_t bytes_per_pixel )
: dds_stream_profile( uid, format, frequency )
dds_video_stream_profile( dds_stream_uid uid, dds_stream_format format, int16_t frequency, uint16_t width, uint16_t height,
uint8_t bytes_per_pixel, int8_t inner_type = 0 )
: dds_stream_profile( uid, format, frequency, inner_type )
, _width( width )
, _height( height )
, _bytes_per_pixel( bytes_per_pixel )
Expand All @@ -146,8 +150,8 @@ class dds_video_stream_profile : public dds_stream_profile
class dds_motion_stream_profile : public dds_stream_profile
{
public:
dds_motion_stream_profile( dds_stream_uid uid, dds_stream_format format, int16_t frequency )
: dds_stream_profile( uid, format, frequency )
dds_motion_stream_profile( dds_stream_uid uid, dds_stream_format format, int16_t frequency, int8_t inner_type = 0 )
: dds_stream_profile( uid, format, frequency, inner_type )
{
}
dds_motion_stream_profile( dds_motion_stream_profile && ) = default;
Expand Down
43 changes: 43 additions & 0 deletions third-party/realdds/include/realdds/dds-subscriber.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// License: Apache 2.0. See LICENSE file in root directory.
// Copyright(c) 2022 Intel Corporation. All Rights Reserved.

#pragma once

#include <memory>


namespace eprosima {
namespace fastdds {
namespace dds {
class Subscriber;
} // namespace dds
} // namespace fastdds
} // namespace eprosima


namespace realdds {


class dds_participant;


// The Subscriber manages the activities of several dds_topic_reader (DataReader) entities
//
class dds_subscriber
{
std::shared_ptr< dds_participant > _participant;

eprosima::fastdds::dds::Subscriber * _subscriber;

public:
dds_subscriber( std::shared_ptr< dds_participant > const & participant );
~dds_subscriber();

eprosima::fastdds::dds::Subscriber * get() const { return _subscriber; }
eprosima::fastdds::dds::Subscriber * operator->() const { return get(); }

std::shared_ptr< dds_participant > const & get_participant() const { return _participant; }
};


} // namespace realdds
8 changes: 6 additions & 2 deletions third-party/realdds/include/realdds/dds-topic-reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,24 @@ namespace realdds {


class dds_topic;

class dds_subscriber;

// The 'reader' is the entity used to subscribe to updated values of data in a topic. It is bound at creation to this
// topic.
//
// You may choose to create one via a 'subscriber' that manages the activities of several readers.
// on_data_available callback will be called when a sample is received.
//
class dds_topic_reader : public eprosima::fastdds::dds::DataReaderListener
{
std::shared_ptr< dds_topic > const _topic;
std::shared_ptr < dds_subscriber > const _subscriber;

eprosima::fastdds::dds::Subscriber * _subscriber = nullptr;
eprosima::fastdds::dds::DataReader * _reader = nullptr;

public:
dds_topic_reader( std::shared_ptr< dds_topic > const & topic );
dds_topic_reader( std::shared_ptr< dds_topic > const & topic, std::shared_ptr< dds_subscriber > const & subscriber );
~dds_topic_reader();

eprosima::fastdds::dds::DataReader * get() const { return _reader; }
Expand Down
56 changes: 56 additions & 0 deletions third-party/realdds/src/dds-control-server.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// License: Apache 2.0. See LICENSE file in root directory.
// Copyright(c) 2022 Intel Corporation. All Rights Reserved.

#include <realdds/dds-control-server.h>

#include <realdds/dds-participant.h>
#include <realdds/dds-subscriber.h>
#include <realdds/dds-stream-server.h>
#include <realdds/dds-utilities.h>
#include <realdds/topics/dds-topics.h>
#include <realdds/dds-topic.h>
#include <realdds/dds-topic-reader.h>

#include <fastdds/dds/topic/Topic.hpp>

#include <fastdds/dds/domain/DomainParticipant.hpp>
#include <fastdds/dds/subscriber/Subscriber.hpp>
#include <fastdds/dds/subscriber/DataReader.hpp>
#include <fastdds/dds/subscriber/DataReaderListener.hpp>
#include <fastdds/dds/subscriber/qos/SubscriberQos.hpp>
#include <librealsense2/utilities/concurrency/concurrency.h>


using namespace eprosima::fastdds::dds;

namespace realdds {


dds_control_server::dds_control_server( std::shared_ptr< dds_subscriber > const & subscriber, const std::string & topic_name )
: _subscriber( subscriber )
{
auto topic = topics::device::control::create_topic( subscriber->get_participant(), topic_name.c_str() );
_reader = std::make_shared< dds_topic_reader >( topic, subscriber );

dds_topic_reader::qos rqos( RELIABLE_RELIABILITY_QOS );
rqos.history().depth = 10; // default is 1
rqos.endpoint().history_memory_policy = eprosima::fastrtps::rtps::DYNAMIC_RESERVE_MEMORY_MODE;
maloel marked this conversation as resolved.
Show resolved Hide resolved
_reader->run( rqos );
}


dds_control_server::~dds_control_server()
{
}


void dds_control_server::on_control_message_received( on_control_message_received_callback callback )
{
if ( !_reader )
DDS_THROW( runtime_error, "setting callback when reader is not created" );
maloel marked this conversation as resolved.
Show resolved Hide resolved

_reader->on_data_available( callback );
}


} // namespace realdds
6 changes: 4 additions & 2 deletions third-party/realdds/src/dds-device-impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ to_realdds_profile( const topics::device::notification::video_stream_profile & p
profile.framerate,
profile.width,
profile.height,
0 ); // TODO - bpp
0, // TODO - bpp
profile.type);
// TODO - add intrinsics

return prof;
Expand All @@ -71,7 +72,8 @@ to_realdds_profile( const topics::device::notification::motion_stream_profile &
{
auto prof = std::make_shared< dds_motion_stream_profile >( dds_stream_uid( profile.uid, profile.stream_index ),
dds_stream_format::from_rs2( profile.format ),
profile.framerate );
profile.framerate,
profile.type );
// TODO - add intrinsics

return prof;
Expand Down
Loading