[libcamera-devel] [RFC v1 2/7] py: Add Python logging category

[Tomi Valkeinen]

On 27/06/2022 11:18, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Mon, Jun 27, 2022 at 09:48:34AM +0300, Tomi Valkeinen wrote:
>> On 24/06/2022 12:50, Laurent Pinchart wrote:
>>> On Thu, Jun 23, 2022 at 05:47:31PM +0300, Tomi Valkeinen wrote:
>>>> Add Python logging category, and use it in handleRequestCompleted().
>>>>
>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
>>>> ---
>>>>    src/py/libcamera/py_main.cpp |  8 ++++++--
>>>>    src/py/libcamera/py_main.h   | 12 ++++++++++++
>>>>    2 files changed, 18 insertions(+), 2 deletions(-)
>>>>    create mode 100644 src/py/libcamera/py_main.h
>>>>
>>>> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
>>>> index 505cc3dc..17b17f60 100644
>>>> --- a/src/py/libcamera/py_main.cpp
>>>> +++ b/src/py/libcamera/py_main.cpp
>>>> @@ -5,6 +5,8 @@
>>>>     * Python bindings
>>>>     */
>>>>    
>>>> +#include "py_main.h"
>>>> +
>>>>    #include <mutex>
>>>>    #include <stdexcept>
>>>>    #include <sys/eventfd.h>
>>>> @@ -23,6 +25,8 @@ namespace py = pybind11;
>>>>    
>>>>    using namespace libcamera;
>>>>    
>>>> +LOG_DEFINE_CATEGORY(Python)
>>>> +
>>>>    template<typename T>
>>>>    static py::object valueOrTuple(const ControlValue &cv)
>>>>    {
>>>> @@ -120,10 +124,10 @@ static void handleRequestCompleted(Request *req)
>>>>    	size_t s = write(gEventfd, &v, 8);
>>>>    	/*
>>>>    	 * We should never fail, and have no simple means to manage the error,
>>>> -	 * so let's use LOG(Fatal).
>>>> +	 * so let's use LOG(Python, Fatal).
>>>
>>> You could also write "so let's log a fatal error".
>>
>> Yep.
>>
>>>>    	 */
>>>>    	if (s != 8)
>>>> -		LOG(Fatal) << "Unable to write to eventfd";
>>>> +		LOG(Python, Fatal) << "Unable to write to eventfd";
>>>>    }
>>>>    
>>>>    void init_py_enums(py::module &m);
>>>> diff --git a/src/py/libcamera/py_main.h b/src/py/libcamera/py_main.h
>>>> new file mode 100644
>>>> index 00000000..1b543a55
>>>> --- /dev/null
>>>> +++ b/src/py/libcamera/py_main.h
>>>> @@ -0,0 +1,12 @@
>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>>>> +/*
>>>> + * Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
>>>> + */
>>>> +
>>>> +#pragma once
>>>> +
>>>> +#include <libcamera/base/log.h>
>>>> +
>>>> +using namespace libcamera;
>>>
>>> "using namespace" directives are discouraged in header files, as they
>>> will propagate to all compilation units. Could this be avoided ? With
>>> that fixed,
>>
>> LOG_DECLARE_CATEGORY() requires either using namespace or being inside
>> libcamera namespace.
>>
>> I guess I could move all the python bindings code to be inside libcamera
>> namespace.
> 
> You can also do
> 
> namespace libcamera {
> 
> LOG_DECLARE_CATEGORY(Python)
> 
> }

That's true. I'll change this as above, and maybe later move the whole 
bindings inside namespace libcamera. Or any concerns with that change?

  Tomi