[libcamera-devel] [RFC v1 6/7] py: New event handling
Tomi Valkeinen
tomi.valkeinen at ideasonboard.com
Mon Jun 27 12:26:48 CEST 2022
On 24/06/2022 13:53, Kieran Bingham wrote:
> Quoting Tomi Valkeinen (2022-06-23 15:47:35)
>> At the moment the Python bindings only handle the requestCompleted
>> events. But we have others, bufferCompleted and disconnec from the
>
> disconnec/disconnect
>
>> Camera, and cameraAdded and cameraRemoved from the CameraManager.
>>
>> This makes all those events available to the users.
>
> \o/
>
>
>> The get_ready_requests() method is now deprecated, but available to keep
>> backward compatibility.
>
> Aha, once again - answering my questions in the previous patch ;-)
> Maybe I need to read patch series backwards ... hehe.
>
>
>> The new event handling works as follows:
>>
>> The user sets callbacks to the CameraManager or Cameras (e.g.
>> Camera.buffer_completed). When the event_fd informs of an event, the
>> user must call CameraManager.dispatch_events() which gets the queued
>> events and calls the relevant callbacks for each queued event.
>>
>> Additionally there is CameraManager.discard_events() if the user does
>> not want to process the events but wants to clear the event queue (e.g.
>> after stopping the cameras or when exiting the app).
>
> Hrm... in C++ lifetimes, the end of stop() means everything internally
> is released, so I'm still weary that if we change that it could be
> problematic (i.e. if we have to 'rely' on an application doing clean
> up).
>
>
>> I'm not very happy with this patch. It works fine, but there's a lot of
>> repetition of almost-the-same code. Perhaps some template magics might
>> reduce the repetition, but I also fear that it can easily make the code
>> more difficult to read.
>>
>> TODO: Documentation.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
>> ---
>> src/py/libcamera/py_camera_manager.cpp | 291 +++++++++++++++++++++++--
>> src/py/libcamera/py_camera_manager.h | 41 +++-
>> src/py/libcamera/py_main.cpp | 87 +++++++-
>> 3 files changed, 397 insertions(+), 22 deletions(-)
>>
>> diff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp
>> index ba45f713..bbadb9ee 100644
>> --- a/src/py/libcamera/py_camera_manager.cpp
>> +++ b/src/py/libcamera/py_camera_manager.cpp
>> @@ -15,6 +15,37 @@ namespace py = pybind11;
>>
>> using namespace libcamera;
>>
>> +struct CameraEvent {
>> + enum class EventType {
>> + Undefined = 0,
>> + CameraAdded,
>> + CameraRemoved,
>> + Disconnect,
>> + RequestCompleted,
>> + BufferCompleted,
>> + };
>> +
>> + CameraEvent(EventType type)
>> + : type(type)
>> + {
>> + }
>> +
>> + EventType type;
>> +
>> + std::shared_ptr<Camera> cam;
>> +
>> + union {
>> + struct {
>> + Request *req;
>> + FrameBuffer *fb;
>> + } buf_completed;
>> +
>> + struct {
>> + Request *req;
>> + } req_completed;
>> + };
>> +};
>> +
>> PyCameraManager::PyCameraManager()
>> {
>> int fd = eventfd(0, 0);
>> @@ -56,33 +87,261 @@ py::list PyCameraManager::getCameras()
>> return l;
>> }
>>
>> +/* DEPRECATED */
>> std::vector<py::object> PyCameraManager::getReadyRequests(bool nonBlocking)
>> {
>> if (!nonBlocking || hasEvents())
>> readFd();
>>
>> - std::vector<Request *> v;
>> - getRequests(v);
>> + std::vector<CameraEvent> v;
>> + getEvents(v);
>>
>> std::vector<py::object> ret;
>>
>> - for (Request *req : v) {
>> - py::object o = py::cast(req);
>> - /* Decrease the ref increased in Camera.queue_request() */
>> - o.dec_ref();
>> - ret.push_back(o);
>> + for (const auto &ev : v) {
>> + switch (ev.type) {
>> + case CameraEvent::EventType::RequestCompleted: {
>> + Request *req = ev.req_completed.req;
>> + py::object o = py::cast(req);
>> + /* Decrease the ref increased in Camera.queue_request() */
>> + o.dec_ref();
>> + ret.push_back(o);
>> + }
>> + default:
>> + /* ignore */
>
> Does this mean that events will get lost if the deprecated call gets
> used? (Ok, so it's deprecated, but will it cause 'bugs' ?)
Lost in what way? They are lost in the sense that the app won't see
them, but that's to be expected as the "legacy" app is not aware of such
a feature.
>
>> + break;
>> + }
>> }
>>
>> return ret;
>> }
>>
>> /* Note: Called from another thread */
>> -void PyCameraManager::handleRequestCompleted(Request *req)
>> +void PyCameraManager::handleBufferCompleted(std::shared_ptr<Camera> cam, Request *req, FrameBuffer *fb)
>> +{
>> + CameraEvent ev(CameraEvent::EventType::BufferCompleted);
>> + ev.cam = cam;
>> + ev.buf_completed.req = req;
>> + ev.buf_completed.fb = fb;
>> +
>> + pushEvent(ev);
>> + writeFd();
>
> Should writeFd be part of pushEvent? then there should be one entry on
> the poll Fd for every event right? And forces them to be kept in sync...
Maybe, but the same way as writeFd() and pushEvent() are a pair, the
readFd() and getEvents() is a pair. But for the latter pair, readFd() is
not always called, so readFd() is separate. Thus I wanted to keep
writeFd() separate too.
So, they could be merged. Let's see how this evolves...
>> +}
>> +
>> +/* Note: Called from another thread */
>> +void PyCameraManager::handleRequestCompleted(std::shared_ptr<Camera> cam, Request *req)
>> +{
>> + CameraEvent ev(CameraEvent::EventType::RequestCompleted);
>> + ev.cam = cam;
>> + ev.req_completed.req = req;
>> +
>> + pushEvent(ev);
>> + writeFd();
>> +}
>> +
>> +/* Note: Called from another thread */
>> +void PyCameraManager::handleDisconnected(std::shared_ptr<Camera> cam)
>> {
>> - pushRequest(req);
>> + CameraEvent ev(CameraEvent::EventType::Disconnect);
>> + ev.cam = cam;
>> +
>> + pushEvent(ev);
>> writeFd();
>> }
>>
>> +/* Note: Called from another thread */
>> +void PyCameraManager::handleCameraAdded(std::shared_ptr<Camera> cam)
>> +{
>> + CameraEvent ev(CameraEvent::EventType::CameraAdded);
>> + ev.cam = cam;
>> +
>> + pushEvent(ev);
>> + writeFd();
>> +}
>> +
>> +/* Note: Called from another thread */
>> +void PyCameraManager::handleCameraRemoved(std::shared_ptr<Camera> cam)
>> +{
>> + CameraEvent ev(CameraEvent::EventType::CameraRemoved);
>> + ev.cam = cam;
>> +
>> + pushEvent(ev);
>> + writeFd();
>> +}
>
> Is that what you mean about code duplication? I think it's fine.
>
> These could be templated out to specify the event type I guess ... but
> that might be more pain. Especially when the request completed has
> different parameters anyway.
>
>> +
>> +void PyCameraManager::dispatchEvents(bool nonBlocking)
>> +{
>> + if (!nonBlocking || hasEvents())
>> + readFd();
>> +
>> + std::vector<CameraEvent> v;
>> + getEvents(v);
>> +
>> + LOG(Python, Debug) << "Dispatch " << v.size() << " events";
>> +
>> + for (const auto &ev : v) {
>> + switch (ev.type) {
>> + case CameraEvent::EventType::CameraAdded: {
>> + std::shared_ptr<Camera> cam = ev.cam;
>> +
>> + if (cameraAddedHandler_)
>> + cameraAddedHandler_(cam);
>> +
>> + break;
>> + }
>> + case CameraEvent::EventType::CameraRemoved: {
>> + std::shared_ptr<Camera> cam = ev.cam;
>> +
>> + if (cameraRemovedHandler_)
>> + cameraRemovedHandler_(cam);
>> +
>> + break;
>> + }
>> + case CameraEvent::EventType::BufferCompleted: {
>> + std::shared_ptr<Camera> cam = ev.cam;
>> +
>> + auto cb = getBufferCompleted(cam.get());
>> +
>> + if (cb)
>> + cb(cam, ev.buf_completed.req, ev.buf_completed.fb);
>> +
>> + break;
>> + }
>> + case CameraEvent::EventType::RequestCompleted: {
>> + std::shared_ptr<Camera> cam = ev.cam;
>> +
>> + auto cb = getRequestCompleted(cam.get());
>> +
>> + if (cb)
>> + cb(cam, ev.req_completed.req);
>> +
>> + /* Decrease the ref increased in Camera.queue_request() */
>> + py::object o = py::cast(ev.req_completed.req);
>> + o.dec_ref();
>> +
>> + break;
>> + }
>> + case CameraEvent::EventType::Disconnect: {
>> + std::shared_ptr<Camera> cam = ev.cam;
>> +
>> + auto cb = getDisconnected(cam.get());
>> +
>> + if (cb)
>> + cb(cam);
>> +
>> + break;
>> + }
>> + default:
>> + assert(false);
>
> Can this be a python throw to generate a backtrace? Or a LOG(Python,
> Fatal)?
>
> A plain assert isn't very informative.
I can change this (and I think there were a few more) asserts to log
fatal. I'm not sure if throwing an exception is good if the error
indicates to some kind of internal bug.
>> + }
>> + }
>> +}
>> +
>> +void PyCameraManager::discardEvents()
>> +{
>> + if (hasEvents())
>> + readFd();
>> +
>> + std::vector<CameraEvent> v;
>> + getEvents(v);
>> +
>> + LOG(Python, Debug) << "Discard " << v.size() << " events";
>> +
>> + for (const auto &ev : v) {
>> + if (ev.type != CameraEvent::EventType::RequestCompleted)
>> + continue;
>> +
>> + std::shared_ptr<Camera> cam = ev.cam;
>> +
>> + /* Decrease the ref increased in Camera.queue_request() */
>> + py::object o = py::cast(ev.req_completed.req);
>> + o.dec_ref();
>> + }
>> +}
>> +
>> +std::function<void(std::shared_ptr<Camera>)> PyCameraManager::getCameraAdded() const
>> +{
>> + return cameraAddedHandler_;
>> +}
>> +
>> +void PyCameraManager::setCameraAdded(std::function<void(std::shared_ptr<Camera>)> fun)
>> +{
>> + if (cameraAddedHandler_)
>> + cameraAdded.disconnect();
>> +
>> + cameraAddedHandler_ = fun;
>> +
>> + if (fun)
>
> Really trivially, but I'd call fun fn or func. Otherwise I read this as
> "If you're having fun... connect the signal" ;-)
Sigh... Ok... =)
>
>> + cameraAdded.connect(this, &PyCameraManager::handleCameraAdded);
>> +}
>> +
>> +std::function<void(std::shared_ptr<Camera>)> PyCameraManager::getCameraRemoved() const
>> +{
>> + return cameraRemovedHandler_;
>> +}
>> +
>> +void PyCameraManager::setCameraRemoved(std::function<void(std::shared_ptr<Camera>)> fun)
>> +{
>> + if (cameraRemovedHandler_)
>> + cameraRemoved.disconnect();
>> +
>> + cameraRemovedHandler_ = fun;
>> +
>> + if (fun)
>> + cameraRemoved.connect(this, &PyCameraManager::handleCameraRemoved);
>> +}
>> +
>> +std::function<void(std::shared_ptr<Camera>, Request *)> PyCameraManager::getRequestCompleted(Camera *cam)
>> +{
>> + if (auto it = cameraRequestCompletedHandlers_.find(cam);
>> + it != cameraRequestCompletedHandlers_.end())
>> + return it->second;
>> +
>> + return nullptr;
>> +}
>> +
>> +void PyCameraManager::setRequestCompleted(Camera *cam, std::function<void(std::shared_ptr<Camera>, Request *)> fun)
>> +{
>> + if (fun)
>> + cameraRequestCompletedHandlers_[cam] = fun;
>> + else
>> + cameraRequestCompletedHandlers_.erase(cam);
>> +}
>> +
>> +std::function<void(std::shared_ptr<Camera>, Request *, FrameBuffer *)> PyCameraManager::getBufferCompleted(Camera *cam)
>> +{
>> + if (auto it = cameraBufferCompletedHandlers_.find(cam);
>> + it != cameraBufferCompletedHandlers_.end())
>> + return it->second;
>> +
>> + return nullptr;
>> +}
>> +
>> +void PyCameraManager::setBufferCompleted(Camera *cam, std::function<void(std::shared_ptr<Camera>, Request *, FrameBuffer *)> fun)
>> +{
>> + if (fun)
>> + cameraBufferCompletedHandlers_[cam] = fun;
>> + else
>> + cameraBufferCompletedHandlers_.erase(cam);
>> +}
>> +
>> +std::function<void(std::shared_ptr<Camera>)> PyCameraManager::getDisconnected(Camera *cam)
>> +{
>> + if (auto it = cameraDisconnectHandlers_.find(cam);
>> + it != cameraDisconnectHandlers_.end())
>> + return it->second;
>> +
>> + return nullptr;
>> +}
>> +
>> +void PyCameraManager::setDisconnected(Camera *cam, std::function<void(std::shared_ptr<Camera>)> fun)
>> +{
>> + if (fun)
>> + cameraDisconnectHandlers_[cam] = fun;
>> + else
>> + cameraDisconnectHandlers_.erase(cam);
>> +}
>> +
>
> Yes, I see, lots more boilerplate code duplication. Not a problem in
> it's own right, and if it's templateable then maybe that can be helpful.
Right. The callback functions are different, but these are so similar
that something must be possible. But I fear templates will make the code
more difficult to read and maintain.
>> void PyCameraManager::writeFd()
>> {
>> uint64_t v = 1;
>> @@ -104,16 +363,18 @@ void PyCameraManager::readFd()
>> throw std::system_error(errno, std::generic_category());
>> }
>>
>> -void PyCameraManager::pushRequest(Request *req)
>> +void PyCameraManager::pushEvent(const CameraEvent &ev)
>> {
>> - std::lock_guard guard(reqlist_mutex_);
>> - reqList_.push_back(req);
>> + std::lock_guard guard(reqlistMutex_);
>> + cameraEvents_.push_back(ev);
>> +
>> + LOG(Python, Debug) << "Queued events: " << cameraEvents_.size();
>> }
>>
>> -void PyCameraManager::getRequests(std::vector<Request *> &v)
>> +void PyCameraManager::getEvents(std::vector<CameraEvent> &v)
>> {
>> - std::lock_guard guard(reqlist_mutex_);
>> - swap(v, reqList_);
>> + std::lock_guard guard(reqlistMutex_);
>
> Is this now supposed to be an eventListMutex_?
Yes. Or rather, cameraEventsMutex_
>
>> + swap(v, cameraEvents_);
>> }
>>
>> bool PyCameraManager::hasEvents()
>> diff --git a/src/py/libcamera/py_camera_manager.h b/src/py/libcamera/py_camera_manager.h
>> index 2396d236..fd28291b 100644
>> --- a/src/py/libcamera/py_camera_manager.h
>> +++ b/src/py/libcamera/py_camera_manager.h
>> @@ -13,6 +13,8 @@
>>
>> using namespace libcamera;
>>
>> +struct CameraEvent;
>> +
>> class PyCameraManager : public CameraManager
>> {
>> public:
>> @@ -27,15 +29,46 @@ public:
>>
>> void handleRequestCompleted(Request *req);
>>
>> + void handleBufferCompleted(std::shared_ptr<Camera> cam, Request *req, FrameBuffer *fb);
>> + void handleRequestCompleted(std::shared_ptr<Camera> cam, Request *req);
>> + void handleDisconnected(std::shared_ptr<Camera> cam);
>> + void handleCameraAdded(std::shared_ptr<Camera> cam);
>> + void handleCameraRemoved(std::shared_ptr<Camera> cam);
>
> In libcamera, we don't prefix event handlers with 'handle'. i.e.
> handleCameraRemoved would just be cameraRemoved.
>
> Do you prefer it to make it distinct? or is it possible to align with
> the existing styles?
We inherit CameraManager, which already has cameraRemoved... And even if
it didn't, isn't "cameraRemoved" a bad name for a function that handles
an event? Shouldn't functions be about doing something?
If it's just for signals and their handlers, isn't it bad there too? A
signal would be "cameraRemoved" and the handler would be
"cameraRemoved", right?
>> +
>> + void dispatchEvents(bool nonBlocking = false);
>> + void discardEvents();
>> +
>> + std::function<void(std::shared_ptr<Camera>)> getCameraAdded() const;
>> + void setCameraAdded(std::function<void(std::shared_ptr<Camera>)> fun);
>> +
>> + std::function<void(std::shared_ptr<Camera>)> getCameraRemoved() const;
>> + void setCameraRemoved(std::function<void(std::shared_ptr<Camera>)> fun);
>> +
>> + std::function<void(std::shared_ptr<Camera>, Request *)> getRequestCompleted(Camera *cam);
>> + void setRequestCompleted(Camera *cam, std::function<void(std::shared_ptr<Camera>, Request *)> fun);
>> +
>> + std::function<void(std::shared_ptr<Camera>, Request *, FrameBuffer *)> getBufferCompleted(Camera *cam);
>> + void setBufferCompleted(Camera *cam, std::function<void(std::shared_ptr<Camera>, Request *, FrameBuffer *)> fun);
>> +
>> + std::function<void(std::shared_ptr<Camera>)> getDisconnected(Camera *cam);
>> + void setDisconnected(Camera *cam, std::function<void(std::shared_ptr<Camera>)> fun);
>
> so many getters and setters. Can the underlying events be more directly
> accessibly or do they have to have the setters?
I felt it makes the main bindings, in py_main.cpp cleaner if we don't
expose data structures from the PyCameraManager and force the
py_main.cpp to do the work with them.
>> +
>> private:
>> int eventFd_ = -1;
>> - std::mutex reqlist_mutex_;
>> + std::mutex reqlistMutex_;
>> std::vector<Request *> reqList_;
>> + std::vector<CameraEvent> cameraEvents_;
>> +
>> + std::function<void(std::shared_ptr<Camera>)> cameraAddedHandler_;
>> + std::function<void(std::shared_ptr<Camera>)> cameraRemovedHandler_;
>> +
>> + std::map<Camera *, std::function<void(std::shared_ptr<Camera>, Request *, FrameBuffer *)>> cameraBufferCompletedHandlers_;
>> + std::map<Camera *, std::function<void(std::shared_ptr<Camera>, Request *)>> cameraRequestCompletedHandlers_;
>> + std::map<Camera *, std::function<void(std::shared_ptr<Camera>)>> cameraDisconnectHandlers_;
>>
>> void writeFd();
>> void readFd();
>> - void pushRequest(Request *req);
>> - void getRequests(std::vector<Request *> &v);
>> -
>> + void pushEvent(const CameraEvent &ev);
>> + void getEvents(std::vector<CameraEvent> &v);
>> bool hasEvents();
>> };
>> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
>> index ee4ecb9b..b6fd9a9d 100644
>> --- a/src/py/libcamera/py_main.cpp
>> +++ b/src/py/libcamera/py_main.cpp
>> @@ -103,8 +103,20 @@ PYBIND11_MODULE(_libcamera, m)
>> .def_property_readonly("cameras", &PyCameraManager::getCameras)
>>
>> .def_property_readonly("event_fd", &PyCameraManager::eventFd)
>> + /* DEPRECATED */
>> .def("get_ready_requests", &PyCameraManager::getReadyRequests,
>> - py::arg("nonblocking") = false);
>> + py::arg("nonblocking") = false)
>> + .def("dispatch_events", &PyCameraManager::dispatchEvents,
>> + py::arg("nonblocking") = false)
>
> Why are there newlines between camera_added and camera_removed, but not
> between get_ready_requests, dispatch_events, and discard_events? Is it
> grouping events? or should these be separated by newlines too?
No particular reason. I often try to group things, or separate things,
but it's not always quite consistent (if it even could be).
>
>> + .def("discard_events", &PyCameraManager::discardEvents)
>> +
>> + .def_property("camera_added",
>> + &PyCameraManager::getCameraAdded,
>> + &PyCameraManager::setCameraAdded)
>> +
>> + .def_property("camera_removed",
>> + &PyCameraManager::getCameraRemoved,
>> + &PyCameraManager::setCameraRemoved);
>
> For these chained code styles, if the ; was on a new line by
> itself, it wouldn't have to modify the previous property to add a new
> one?
>
> Is that a good thing or worse code style for you ?
That's what I had originally, but I dropped it either because of a
review comment, or because clang-format very much wants to move the ; to
the end of the line. Or maybe a style checker warned about it.
>
>> pyCamera
>> .def_property_readonly("id", &Camera::id)
>> @@ -117,9 +129,13 @@ PYBIND11_MODULE(_libcamera, m)
>> auto cm = gCameraManager.lock();
>> assert(cm);
>>
>> - self.requestCompleted.connect(&self, [cm=std::move(cm)](Request *req) {
>> + /*
>> + * Note: We always subscribe requestComplete, as the bindings
>> + * use requestComplete event to decrement the Request refcount-
>> + */
>
> Interesting, so there are multiple subscribers to the event? Is it still
Just a single subscriber. The comment above describes why this one,
requestComplete, is different than the other events. For the other
events the bindings are just passing them through. But the bindings
require requestComplete, so it's always implicitly subscribed.
Or maybe you mean something else, but if so, I didn't get your point =).
> guaranteed to be referenced while the user code has the request? (I
> assume so, and that this is managing the internal refcnts while the
> object is inside libcamera).
>
>
> If the ordering is required, then this layer could subscribe to the
> event, and the be responsible for calling the event on the user code and
> cleaning up after it completes.
>
> But if it already is fine, then it's a nice example of how multiple
> functions can run on the event handler.
>
>>
>> - cm->handleRequestCompleted(req);
>> + self.requestCompleted.connect(&self, [cm, camera=self.shared_from_this()](Request *req) {
>> + cm->handleRequestCompleted(camera, req);
>> });
>>
>> ControlList controlList(self.controls());
>> @@ -148,6 +164,71 @@ PYBIND11_MODULE(_libcamera, m)
>> return 0;
>> })
>>
>> + .def_property("request_completed",
>> + [](Camera &self) {
>> + auto cm = gCameraManager.lock();
>> + assert(cm);
>> +
>> + return cm->getRequestCompleted(&self);
>> + },
>> + [](Camera &self, std::function<void(std::shared_ptr<Camera>, Request *)> f) {
>> + auto cm = gCameraManager.lock();
>> + assert(cm);
>> +
>> + cm->setRequestCompleted(&self, f);
>> +
>> + /*
>> + * Note: We do not subscribe requestComplete here, as we
>> + * do that in the start() method.
>> + */
>> + })
>
> Ok - so this shows why they all need distince getters and setters ...
>
>
>> +
>> + .def_property("buffer_completed",
>> + [](Camera &self) -> std::function<void(std::shared_ptr<Camera>, Request *, FrameBuffer *)> {
>> + auto cm = gCameraManager.lock();
>> + assert(cm);
>> +
>> + return cm->getBufferCompleted(&self);
>> + },
>> + [](Camera &self, std::function<void(std::shared_ptr<Camera>, Request *, FrameBuffer *)> f) {
>> + auto cm = gCameraManager.lock();
>> + assert(cm);
>> +
>> + cm->setBufferCompleted(&self, f);
>> +
>> + self.bufferCompleted.disconnect();
>
> Can a comment explain why we're disconnecting this bufferCompleted? It's
> not obvious to me why it's there ... Is it an internal bufferCompleted
> handler for some specificness that I haven't seen yet?
When the user sets the callback function, like
cam.buffer_completed = foobar
we store the callback (cm->setBufferCompleted()), we then disconnect any
previously connected handler, and then connect the new handler (unless !f).
So the disconnect is just disconnecting (possibly) previously set handler.
However, that signal handler lambda below is always the same (we don't
pass the 'f' from the user), so we don't actually need to reconnect if
already connected. So this could be optimized a bit by
- disconnecting only if we have connected earlier and !f
- connecting only if not connected earlier and f
But it may not be worth it, especially as we don't have the "have we
already connected?" available here.
>> +
>> + if (!f)
>> + return;
>> +
>> + self.bufferCompleted.connect(&self, [cm, camera=self.shared_from_this()](Request *req, FrameBuffer *fb) {
>> + cm->handleBufferCompleted(camera, req, fb);
>> + });
>> + })
>> +
>> + .def_property("disconnected",
>> + [](Camera &self) -> std::function<void(std::shared_ptr<Camera>)> {
>> + auto cm = gCameraManager.lock();
>> + assert(cm);
>> +
>> + return cm->getDisconnected(&self);
>> + },
>> + [](Camera &self, std::function<void(std::shared_ptr<Camera>)> f) {
>> + auto cm = gCameraManager.lock();
>> + assert(cm);
>> +
>> + cm->setDisconnected(&self, f);
>> +
>> + self.disconnected.disconnect();
>
> Same here. Are there internal events just to be able to run by default?
I don't think I understand the question. What do you mean with internal
events? The requestCompleted is the only one that has special handling.
>> +
>> + if (!f)
>> + return;
>
> Should the self.disconnected be reconnected if f was null?
Hmm, no... If the f is null, we don't want to see any disconnect events.
Maybe the confusion comes from the layered signal handling.
The bindings connect the C++ signals to its internal handlers. These
handlers store the events. Later the bindings use stored Python callback
functions to deliver the events.
So e.g.
cm->setDisconnected(&self, f);
above stores the Python callback, it doesn't deal with the C++
signals/handling in any way.
>> +
>> + self.disconnected.connect(&self, [cm, camera=self.shared_from_this()]() {
>> + cm->handleDisconnected(camera);
>> + });
>> + })
>> +
>> .def("__str__", [](Camera &self) {
>> return "<libcamera.Camera '" + self.id() + "'>";
>> })
>> --
>> 2.34.1
>>
More information about the libcamera-devel
mailing list