[libcamera-devel] [PATCH 3/3] Documentation: Update the "Start an event loop" section

[Daniel Semkowicz]

The description sounds clear. I would just add minor changes.

On Fri, Jun 17, 2022 at 10:01 AM Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> Hi Daniel,
>
> On Fri, Jun 17, 2022 at 09:24:50AM +0200, Daniel Semkowicz wrote:
> > Hi Jacopo,
> >
> > On Thu, Jun 16, 2022 at 1:06 PM Jacopo Mondi <jacopo at jmondi.org> wrote:
> > >
> > > Hi Daniel,
> > >
> > >    I wonder if we shouldn't just drop the whole
> > >
> > > Start an event loop
> > > ~~~~~~~~~~~~~~~~~~~
> > >
> > > paragraph
> > >
> >
> > I was also thinking about that when I was editing the current text.
> > It is always the decision on how many details about the internal
> > architecture We want to present to the developer in this guide.
> > As it is the introduction guide then maybe indeed it will be better to
> > keep it simple and do not talk too much about the internals.
> >
> > There will still be this one "sleep_for(3000ms)" line needed and I think
> > it would be good to leave some short explanation why it is here. Do you
> > think that adding it at the end of "Request queueing" section is a good
> > idea or should I create a separate section about it?
>
> You're right, throwing a sleep_for() without saying anything might be
> not the best idea :)
>
> Let's resume your original proposal and let me try to integrate it
> with a full rewrite of the paragraph.
>
> >
> > > > -Start an event loop
> > > > +Event loop
> > > >  ~~~~~~~~~~~~~~~~~~~
> > > >
> > > >  The libcamera library needs an event loop to monitor and dispatch events
> > > > @@ -524,17 +526,17 @@ a notifier it is monitoring, it emits the notifier's
> > > >  notifiers' signals and emit application visible events, such as the
> > > >  ``Camera::bufferReady`` and ``Camera::requestCompleted`` signals.
> > > >
> > > > -The code below retrieves a reference to the system-wide event dispatcher and for
> > > > -the a fixed duration of 3 seconds, processes all the events detected in the
> > > > -system.
> > > > +Event loop is handled internally by ``CameraManager`` instance in a separate
> > > > +thread. ``CameraManager::start()`` creates a new thread and starts the event
> > > > +loop processing.
>
> Event loop
> ~~~~~~~~~~~~~~~~~~~
>
> The libcamera library needs an event loop to monitor and dispatch
> events generated by the video devices part of the capture pipeline.
> libcamera runs its own event loop in a separate thread, created at
> ``CameraManager::start()_`` time. This means applications will run in
> their own thread and need to manage their own execution opportunely,
> to respond to user generate events and to libcamera generated events
> emitted through signals.

typo: "user generate" -> "user generated"

>
> Real-world applications will likely either integrate in the event loop
> of higher level frameworks or create their own one to respond to user
> generated events, while for the simple application presented in this
> example it is enough to prevent immediate termination by pausing for 3
> seconds, during which the asynchronous request completion events
> generated by libcamera will be handled by the ``requestComplete()``
> slot connected to the ``Camera::requestCompleted`` signal.

I would break down this part into less complex sentences to improve
readability. Something like:

Real-world applications will likely either integrate in the event loop
of higher level frameworks or create their own one to respond to user
generated events. For the simple application presented in this example,
it is enough to prevent immediate termination by pausing for 3 seconds.
During this time, the asynchronous request completion events generated
by libcamera will be handled by the ``requestComplete()`` slot connected
to the ``Camera::requestCompleted`` signal.

Is it ok?

Best regards
Daniel

>
> .. code:: cpp
>
>    std::this_thread::sleep_for(3000ms);
>
> What do you think ?
>
> > > > +
> > > > +As the ``CameraManager`` was already started in our example, we need to prevent
> > > > +the immediate termination of the application. The code below pauses the main
> > > > +thread for 3 seconds, so that the event loop thread can process the requests.
> > > >
> > > >  .. code:: cpp
> > > >
> > > > -   EventDispatcher *dispatcher = cm->eventDispatcher();
> > > > -   Timer timer;
> > > > -   timer.start(3000);
> > > > -   while (timer.isRunning())
> > > > -       dispatcher->processEvents();
> > > > +   std::this_thread::sleep_for(3000ms);
> > > >
> > > >  Clean up and stop the application
> > > >  ---------------------------------
> > > > --
> > > > 2.34.1
> > > >