[libcamera-devel] py: Fix CameraManager.version property
diff mbox series

Message ID 20230330173956.417714-1-tomi.valkeinen@ideasonboard.com
State Accepted
Headers show
Series
  • [libcamera-devel] py: Fix CameraManager.version property
Related show

Commit Message

Tomi Valkeinen March 30, 2023, 5:39 p.m. UTC
The current CameraManager.version doesn't work at all (raises a
TypeError), as that's not how you use expose C++ static methods as
Python class methods.

Fix it.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reported-by: @meawoppl:matrix.org
---
 src/py/libcamera/py_main.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Laurent Pinchart April 5, 2023, 4:51 a.m. UTC | #1
Hi Tomi,

Thank you for the patch.

On Thu, Mar 30, 2023 at 08:39:56PM +0300, Tomi Valkeinen via libcamera-devel wrote:
> The current CameraManager.version doesn't work at all (raises a
> TypeError), as that's not how you use expose C++ static methods as
> Python class methods.
> 
> Fix it.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> Reported-by: @meawoppl:matrix.org

We can't include people without an e-mail address into the conversation,
defeating the point of the Reported-by tag a bit. I'm inclined to drop
it.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/py/libcamera/py_main.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
> index d14e18e2..c55495cc 100644
> --- a/src/py/libcamera/py_main.cpp
> +++ b/src/py/libcamera/py_main.cpp
> @@ -105,7 +105,7 @@ PYBIND11_MODULE(_libcamera, m)
>  			return cm;
>  		})
>  
> -		.def_property_readonly("version", &PyCameraManager::version)
> +		.def_property_readonly_static("version", [](py::object /* self */) { return PyCameraManager::version(); })
>  		.def("get", &PyCameraManager::get, py::keep_alive<0, 1>())
>  		.def_property_readonly("cameras", &PyCameraManager::cameras)
>
Laurent Pinchart April 5, 2023, 4:53 a.m. UTC | #2
On Wed, Apr 05, 2023 at 07:51:03AM +0300, Laurent Pinchart via libcamera-devel wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Thu, Mar 30, 2023 at 08:39:56PM +0300, Tomi Valkeinen via libcamera-devel wrote:
> > The current CameraManager.version doesn't work at all (raises a
> > TypeError), as that's not how you use expose C++ static methods as
> > Python class methods.
> > 
> > Fix it.
> > 
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > Reported-by: @meawoppl:matrix.org
> 
> We can't include people without an e-mail address into the conversation,
> defeating the point of the Reported-by tag a bit. I'm inclined to drop
> it.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Also, could you extend the python bindings unit test to access
CameraManager.verion ?

> > ---
> >  src/py/libcamera/py_main.cpp | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
> > index d14e18e2..c55495cc 100644
> > --- a/src/py/libcamera/py_main.cpp
> > +++ b/src/py/libcamera/py_main.cpp
> > @@ -105,7 +105,7 @@ PYBIND11_MODULE(_libcamera, m)
> >  			return cm;
> >  		})
> >  
> > -		.def_property_readonly("version", &PyCameraManager::version)
> > +		.def_property_readonly_static("version", [](py::object /* self */) { return PyCameraManager::version(); })
> >  		.def("get", &PyCameraManager::get, py::keep_alive<0, 1>())
> >  		.def_property_readonly("cameras", &PyCameraManager::cameras)
> >

Patch
diff mbox series

diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
index d14e18e2..c55495cc 100644
--- a/src/py/libcamera/py_main.cpp
+++ b/src/py/libcamera/py_main.cpp
@@ -105,7 +105,7 @@  PYBIND11_MODULE(_libcamera, m)
 			return cm;
 		})
 
-		.def_property_readonly("version", &PyCameraManager::version)
+		.def_property_readonly_static("version", [](py::object /* self */) { return PyCameraManager::version(); })
 		.def("get", &PyCameraManager::get, py::keep_alive<0, 1>())
 		.def_property_readonly("cameras", &PyCameraManager::cameras)