| Message ID | 20220930121812.5768-1-david.plowman@raspberrypi.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Quoting David Plowman via libcamera-devel (2022-09-30 13:18:12) > Such controls can now be created when a control doesn't have a > reasonable or obvious default value. We support them using Python's > "None" value, rather than generating a runtime error. > This sounds correct and reasonable to me. This looks like the sort of thing that should have a unit test. I assume you're testing it with picamera2 already though. Tomi, Have we got anywhere we can add tests for things like this ? Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > --- > src/py/libcamera/py_helpers.cpp | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/src/py/libcamera/py_helpers.cpp b/src/py/libcamera/py_helpers.cpp > index 45aecce9..f685e606 100644 > --- a/src/py/libcamera/py_helpers.cpp > +++ b/src/py/libcamera/py_helpers.cpp > @@ -55,6 +55,7 @@ py::object controlValueToPy(const ControlValue &cv) > return py::cast(v); > } > case ControlTypeNone: > + return py::none(); > default: > throw std::runtime_error("Unsupported ControlValue type"); > } > @@ -91,6 +92,7 @@ ControlValue pyToControlValue(const py::object &ob, ControlType type) > case ControlTypeSize: > return ControlValue(ob.cast<Size>()); > case ControlTypeNone: > + return ControlValue(); > default: > throw std::runtime_error("Control type not implemented"); > } > -- > 2.30.2 >
Hi David, Returning "ControlTypeNone" was intentional to signal that no default value exists. This is much more clear and expressive than returning a default constructed 0. So returning "None" on the Python side makes a lot of sense. Reviewed-by: Christian Rauch <Rauch.Christian@gmx.de> Am 30.09.22 um 14:18 schrieb David Plowman via libcamera-devel: > Such controls can now be created when a control doesn't have a > reasonable or obvious default value. We support them using Python's > "None" value, rather than generating a runtime error. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > --- > src/py/libcamera/py_helpers.cpp | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/src/py/libcamera/py_helpers.cpp b/src/py/libcamera/py_helpers.cpp > index 45aecce9..f685e606 100644 > --- a/src/py/libcamera/py_helpers.cpp > +++ b/src/py/libcamera/py_helpers.cpp > @@ -55,6 +55,7 @@ py::object controlValueToPy(const ControlValue &cv) > return py::cast(v); > } > case ControlTypeNone: > + return py::none(); > default: > throw std::runtime_error("Unsupported ControlValue type"); > } > @@ -91,6 +92,7 @@ ControlValue pyToControlValue(const py::object &ob, ControlType type) > case ControlTypeSize: > return ControlValue(ob.cast<Size>()); > case ControlTypeNone: > + return ControlValue(); > default: > throw std::runtime_error("Control type not implemented"); > }
On 30/09/2022 18:37, Kieran Bingham via libcamera-devel wrote: > Quoting David Plowman via libcamera-devel (2022-09-30 13:18:12) >> Such controls can now be created when a control doesn't have a >> reasonable or obvious default value. We support them using Python's >> "None" value, rather than generating a runtime error. >> > > This sounds correct and reasonable to me. I agree. Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > This looks like the sort of thing that should have a unit test. I assume > you're testing it with picamera2 already though. > > Tomi, Have we got anywhere we can add tests for things like this ? We have test/py/unittests.py, although we probably want to split it up to multiple files if we start adding more. Tomi
diff --git a/src/py/libcamera/py_helpers.cpp b/src/py/libcamera/py_helpers.cpp index 45aecce9..f685e606 100644 --- a/src/py/libcamera/py_helpers.cpp +++ b/src/py/libcamera/py_helpers.cpp @@ -55,6 +55,7 @@ py::object controlValueToPy(const ControlValue &cv) return py::cast(v); } case ControlTypeNone: + return py::none(); default: throw std::runtime_error("Unsupported ControlValue type"); } @@ -91,6 +92,7 @@ ControlValue pyToControlValue(const py::object &ob, ControlType type) case ControlTypeSize: return ControlValue(ob.cast<Size>()); case ControlTypeNone: + return ControlValue(); default: throw std::runtime_error("Control type not implemented"); }
Such controls can now be created when a control doesn't have a reasonable or obvious default value. We support them using Python's "None" value, rather than generating a runtime error. Signed-off-by: David Plowman <david.plowman@raspberrypi.com> --- src/py/libcamera/py_helpers.cpp | 2 ++ 1 file changed, 2 insertions(+)