Mavsdk: DroneCore handles only one component

[shakthi-prashanth-m]

While working on Camera plugin, I realized that DroneCore core library can communicate with one component of a vehicle only.
For.eg: A Drone has components such as Autopilot, Camera, Gimbal, etc. Each component has a unique Component ID, but System ID / UUID is same for all.

As per the MAVLink protocol design, all the components send Heartbeat indicating they're ready for communication. DroneCore core library maintains a map of devices that are discovered in the process.
When autopilot component of a vehicle is discovered, it is added to the map. However when a another component (say, camera) of the same system/vehicle is discovered, it won't add it to the map.

Due to this model, DroneCore can only interact with one component of a system only. Suppose I would like to interact with a camera component and autopilot as well, I can only think of creating another instance of DroneCore. Is that recommended ?

If we design core to handle multiple components, camera plugin may be used as below:

DroneCore dc;
dc.add_udp_connection(14540); // same port can be used for both Autopilot & Camera as well

uint64_t uuid;
uint8_t drone_id, camera_id;
dc.register_on_discover(std::bind(&on_drone_discover, _1, _2)); // UUID, Comp ID
dc.register_on_discover(std::bind(&on_camera_discover, _1, _2)); // UUID, Comp ID

while (!dc.is_autopilot_connected(drone_id) &&
!dc.is_camera_connected(camera_id)) {
sleep_for(seconds(1);
}

Device &camera = dc.get_device(uuid, camera_id);
camera->start_video_capture();

Device &drone = dc.get_device(uuid, drone_id);
auto offboard = std::make_shared<Offboard>(&drone);
offboard->set_velocity_ned(......);
....

camera->stop_video_capture();

Right now, camera plugin should handle all the discover, connect logic in itself.

As already discussed that DroneCore is also meant for dealing with any component, how about adding these APIs ?

bool Device::is_autopilot(uint64_t uuid, uint8_t comp_id);
bool Device::is_camera(uint64_t uuid, uint8_t comp_id);
std::vector<unit8_t> DroneCore::get_camera_ids(uint64_t uuid);

So, we can maintain many manage multiple camera devices of a drone as well.

Also related to #292

[julianoes]

@shakthi-prashanth-m that's a good point. We should support a system with multiple components.

I disagree with these API's though because the component ids don't really need to be exposed.
The component IDs are defined by mavlink, so we can handle it internally.

[hamishwillee]

I can't believe MAVLink are only letting us have six cameras https://mavlink.io/en/messages/common.html#MAV_COMP_ID_CAMERA :-)

[shakthi-prashanth-m]

@julianoes

I disagree with these API's though because the component ids don't really need to be exposed.

If application is interested in a particular camera (say, downward facing camera), then it needs to be differentiated it by unique camera ID. Can't we use component IDs as camera IDs to differentiate ?

Suppose that a system(vehicle) has 6 cameras, an application may access to camera 1 & 2 as below:

DroneCore dc;
...
auto camera_ids = dc.camera_ids(); // similar to device_uuids() method.

// Assuming we've have Camera plugin which is instantiated as below:
auto camera1 = std::make_shared<Camera>(&dc.camera(camera_ids[0]); // access to camera 1
auto camera2 = std::make_shared<Camera>(&dc.camera(camera_ids[1]); // access to camera 2
...

[julianoes]

I think the API needs to be as simple and clean as possible and therefore I push back on complexity unless absolutely needed. DroneCore could allow to access different cameras but it should be an abstracted API and not the mavlink one.

[shakthi-prashanth-m]

Ok I agree to keep API as much simple possible. I want to understand what is the complexity in above code.
With current DroneCore interface, we query the device UUIDs that are connected. As we know UUID is per vehicle or system, we need a scheme to distinguish components in a system. If you prefer not to use MAVLink component id, what is the alternative?
We query the devices by UUID. We need to change this so that we get access to the exact component of the system.
So application should make a first call to get UUID list and then make another to get all the components of it.
This is what the above code is trying to do. Do you have a better idea ?

[shakthi-prashanth-m]

DroneCore dc;
auto ddids = dc.device_ids(); 

Device &device1 = dc.device(ddids[0]);
// Assuming we've have Camera plugin which is instantiated as below:
auto camera1 = std::make_shared<Camera>(&device1); // access to camera 1
// camera1 operations 

Device &device2 = dc.device(ddids[1]);
auto camera2 = std::make_shared<Camera>(&device2); // access to camera 2
// camera2 operations 
...

Is this better ?

[julianoes]

Let's follow up with the discussion in #310, this is getting confusing.