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

Update extending.md #30

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Update extending.md #30

wants to merge 2 commits into from

Conversation

lbegani
Copy link

@lbegani lbegani commented Aug 13, 2018

Add details on plugins for camera device

Add details on plugins for camera device
@@ -15,39 +15,35 @@ Video Streaming | Developers can implement new types of video streaming (e.g. HL

## Support Custom Camera Device

In order to support a new type of camera a custom class must be derived from `CameraDevice`. In addition, code must be added to detect the new device type and to instantiate the new custom camera device when this happens. The sections below detail these steps.
Developers may want to add support for a new type of camera device that is not currently supported by DCM. To support a custom type of camera, a `PluginXXX` class derived from `PluginBase` and `CameraDeviceXXX` class derived from `CameraDevice` must be implemented. An example of a plugin for a custom camera device with classes `PluginCustom`(https://github.com/Dronecode/camera-manger/tree/master/plugins) and `CameraDeviceCustom`(https://github.com/Dronecode/camera-manger/tree/master/plugins) is added in the project and may be referenced.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The source code links below don't work (I get 404 so possibly secured).

Copy link
Author

@lbegani lbegani Aug 16, 2018

Choose a reason for hiding this comment

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


### 1. Extend CameraDevice Class
### 1. Extend PluginBase Class
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unless this must be done in order, don't have titles numbered.

To add support for new type of camera device in DCM, a custom class must be derived from `CameraDevice`.
An example `CameraDeviceCustom` (in green) can be seen in the [class diagram](../guide/architecture.md#class_diagram).
The class `CameraDeviceCustom` represents a custom type of camera device.
Plugins are self-registering objects that discovers, enlists and creates camera devices.
Copy link
Collaborator

Choose a reason for hiding this comment

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

that discover, list and, ...

The class `CameraDeviceCustom` represents a custom type of camera device.
Plugins are self-registering objects that discovers, enlists and creates camera devices.

**Action**: `PluginCustom` must implement the pure virtual functions of the base class `PluginBase`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure we need to have these "Action" headings. It might also be worth having the functions that must be implemented listed here.

int setParam(CameraParameters &camParam, std::string param, const char *param_value,
size_t value_size, int param_type);
```
There are few methods in `CameraDevice` class that are not pure virtual. By default it returns `NOT_SUPPORTED`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

By default they return NOT_SUPPORTED.


**Action**: `CameraDeviceCustom` may overload other methods to provide the functionality.
**Action**: `CameraDeviceCustom` may overload not-pure virtual methods to provide the functionality.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, can't it overload any methods it likes ? Both pure and impure?


**Action**: `CameraDeviceCustom` may overload other methods to provide the functionality.
**Action**: `CameraDeviceCustom` may overload not-pure virtual methods to provide the functionality.

The configurable parameters of the custom camera device can be exported to the client (GCS) for control. Setting of these parameters and resetting of all the parameters must be handled by the `CameraDeviceCustom` class.

**Action*: `CameraDeviceCustom` must declare the parameter name (string), ID (int) and type (enum). Also set the default value of the parameter.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again, not sure that we really need the "Action" headings. But if you want to keep them, this one has a missing asterisk.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So maybe something like

CameraDeviceCustom may declare the parameters that are relevant to the camera type. Each parameter has a name (string), ID (int), type (enum) and default value.

What about the case where a camera can have unique parameters? ie it implements some generic API, but might have additional parameters that aren't part of the API itself (ie that might be set by camera config file).
I guess what I'm saying is what parameters get set here, and which get set in a camera definition file?

} else if (camdev_name.find("camera/custom") != std::string::npos) {
return std::make_shared<CameraDeviceCustom>(camdev_name);
```
The `CameraServer` will query the `PluginManager` for the list of camera devices that are detected. `CameraServer` will create the `CameraDevice` if the device is not in the blacklist.
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe link to blacklist docs?

@hamishwillee
Copy link
Collaborator

@lbegani Looks like a good start, but links to custom plugin don't work, so not sure how much can be inferred from source. I'd be tempted to list the methods that need to be overloaded - we were going to document these in source right?

@lbegani
Copy link
Author

lbegani commented Aug 16, 2018

@hamishwillee
Copy link
Collaborator

OK, probably if you link to the base classes that doc will help people understand what is going on! Great.

@hamishwillee
Copy link
Collaborator

hamishwillee commented Aug 16, 2018

@lbegani I have created some experimental API ref here.

  1. Do you think having this in the documentation is useful?
  2. Is this the public API that we need documented? If not, what other classes do you think need to be included - parameters? Should anything be removed?
  3. As you can see there is a lot that is NOT documented there. It is good to provide comments for as much as possible. Some of this is because we haven't tagged comments as @brief - this is a good way to highlight the short description for an element - e.g this has a description http://localhost:4000/en/plugin_ref/struct_camera_data.html#struct_camera_info_1a23844d6d1740e872be7a06eb5884f98d - if you tag it as @brief then the description will also appear up the top.

@mrpollo mrpollo force-pushed the master branch 4 times, most recently from fc20582 to 3af8220 Compare April 29, 2020 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants