[libcamera-devel,v2,2/5] libcamera: device_enumerator: Emit a signal when a new device is hotplugged

Message ID 20200513172950.72685-3-email@uajain.com
State Superseded
Delegated to: Umang Jain
Headers show
Series
  • Introduce UVC hotplug support
Related show

Commit Message

Umang Jain May 13, 2020, 5:29 p.m. UTC
Emit a signal whenever is a new MediaDevice is added to the
DeviceEnumerator. This will allow CameraManager to get notified
about the new devices that have been hot-plugged.

Signed-off-by: Umang Jain <email@uajain.com>
---
 src/libcamera/camera_manager.cpp          |  4 ++--
 src/libcamera/device_enumerator.cpp       | 12 ++++++++++++
 src/libcamera/include/device_enumerator.h |  4 ++++
 3 files changed, 18 insertions(+), 2 deletions(-)

Comments

Kieran Bingham May 18, 2020, 1:20 p.m. UTC | #1
Hi Umang,

On 13/05/2020 18:29, Umang Jain wrote:
> Emit a signal whenever is a new MediaDevice is added to the

s/ whenever is a / whenever a /

> DeviceEnumerator. This will allow CameraManager to get notified

s/get/be/

> about the new devices that have been hot-plugged.
> 
> Signed-off-by: Umang Jain <email@uajain.com>
> ---
>  src/libcamera/camera_manager.cpp          |  4 ++--
>  src/libcamera/device_enumerator.cpp       | 12 ++++++++++++
>  src/libcamera/include/device_enumerator.h |  4 ++++
>  3 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index 1579ad4..a13cfe1 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -154,12 +154,12 @@ void CameraManager::Private::enumerateDevices()
>  		}
>  	}
>  
> -	/* \todo register hot-plug callback here */
> +	enumerator_->newDevicesFound.connect(this, &Private::enumerateDevices);

I'm half tempted to suggest calling this signal hotplug ...

But I think it's just bikesheding as then I end up down the path of
should it be hotPlug or hotplug, and should we have hotUnPlug or
hotplugRemove or ... oh it's a rabbit hole ;-)


>  }
>  
>  void CameraManager::Private::cleanup()
>  {
> -	/* \todo unregister hot-plug callback here */
> +	enumerator_->newDevicesFound.disconnect(this, &Private::enumerateDevices);
>  
>  	/*
>  	 * Release all references to cameras and pipeline handlers to ensure
> diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
> index dd17e3e..0492a86 100644
> --- a/src/libcamera/device_enumerator.cpp
> +++ b/src/libcamera/device_enumerator.cpp
> @@ -227,6 +227,15 @@ std::unique_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &d
>  	return media;
>  }
>  
> +/**

I think this is missing tieing it to the signal?

Not 100% sure how to do that with doxygen, perhaps it's just

 \var newDevicesFound

Presumably other signals are documented to show how this is done.


> +* \brief Notify of new media devices being found
> +*
> +* This signal is emitted when the device enumerator finds new media devices in
> +* the system. It may be emitted for every newly detected device, or once for
> +* multiple of devices, at the discretion of the device enumerator. Not all
> +* device enumerator types may support dynamic detection of new devices.
> +*/
> +
>  /**
>   * \brief Add a media device to the enumerator
>   * \param[in] media media device instance to add
> @@ -242,6 +251,9 @@ void DeviceEnumerator::addDevice(std::unique_ptr<MediaDevice> &&media)
>  		<< "Added device " << media->deviceNode() << ": " << media->driver();
>  
>  	devices_.push_back(std::move(media));
> +
> +	/* \todo: To batch multiple additions, emit with a small delay here. */

Do we have support already to register a timeout or such to make this
call N milliseconds after the last registration of a device?

Sounds like a good way to reduce the signal frequency...

> +	newDevicesFound.emit();
>  }
>  
>  /**
> diff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h
> index 433e357..cd4e846 100644
> --- a/src/libcamera/include/device_enumerator.h
> +++ b/src/libcamera/include/device_enumerator.h
> @@ -13,6 +13,8 @@
>  
>  #include <linux/media.h>
>  
> +#include <libcamera/signal.h>
> +
>  namespace libcamera {
>  
>  class MediaDevice;
> @@ -43,6 +45,8 @@ public:
>  
>  	std::shared_ptr<MediaDevice> search(const DeviceMatch &dm);
>  
> +	Signal<> newDevicesFound;
> +
>  protected:
>  	std::unique_ptr<MediaDevice> createDevice(const std::string &deviceNode);
>  	void addDevice(std::unique_ptr<MediaDevice> &&media);
>

Patch

diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
index 1579ad4..a13cfe1 100644
--- a/src/libcamera/camera_manager.cpp
+++ b/src/libcamera/camera_manager.cpp
@@ -154,12 +154,12 @@  void CameraManager::Private::enumerateDevices()
 		}
 	}
 
-	/* \todo register hot-plug callback here */
+	enumerator_->newDevicesFound.connect(this, &Private::enumerateDevices);
 }
 
 void CameraManager::Private::cleanup()
 {
-	/* \todo unregister hot-plug callback here */
+	enumerator_->newDevicesFound.disconnect(this, &Private::enumerateDevices);
 
 	/*
 	 * Release all references to cameras and pipeline handlers to ensure
diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
index dd17e3e..0492a86 100644
--- a/src/libcamera/device_enumerator.cpp
+++ b/src/libcamera/device_enumerator.cpp
@@ -227,6 +227,15 @@  std::unique_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &d
 	return media;
 }
 
+/**
+* \brief Notify of new media devices being found
+*
+* This signal is emitted when the device enumerator finds new media devices in
+* the system. It may be emitted for every newly detected device, or once for
+* multiple of devices, at the discretion of the device enumerator. Not all
+* device enumerator types may support dynamic detection of new devices.
+*/
+
 /**
  * \brief Add a media device to the enumerator
  * \param[in] media media device instance to add
@@ -242,6 +251,9 @@  void DeviceEnumerator::addDevice(std::unique_ptr<MediaDevice> &&media)
 		<< "Added device " << media->deviceNode() << ": " << media->driver();
 
 	devices_.push_back(std::move(media));
+
+	/* \todo: To batch multiple additions, emit with a small delay here. */
+	newDevicesFound.emit();
 }
 
 /**
diff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h
index 433e357..cd4e846 100644
--- a/src/libcamera/include/device_enumerator.h
+++ b/src/libcamera/include/device_enumerator.h
@@ -13,6 +13,8 @@ 
 
 #include <linux/media.h>
 
+#include <libcamera/signal.h>
+
 namespace libcamera {
 
 class MediaDevice;
@@ -43,6 +45,8 @@  public:
 
 	std::shared_ptr<MediaDevice> search(const DeviceMatch &dm);
 
+	Signal<> newDevicesFound;
+
 protected:
 	std::unique_ptr<MediaDevice> createDevice(const std::string &deviceNode);
 	void addDevice(std::unique_ptr<MediaDevice> &&media);