[libcamera-devel] [PATCH v3 10/17] py: Use exceptions instead of returning error codes
Tomi Valkeinen
tomi.valkeinen at ideasonboard.com
Fri Aug 19 08:43:02 CEST 2022
On 18/08/2022 17:36, Jacopo Mondi wrote:
> Hi Tomi
>
> On Fri, Jul 01, 2022 at 11:45:14AM +0300, Tomi Valkeinen wrote:
>> We have multiple methods which return an error code, mimicing the C++
>> API. Using exceptions is more natural in the Python API, so change all
>> those methods to raise an Exception instead.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
>> ---
>> src/py/cam/cam.py | 16 +----
>> src/py/examples/simple-cam.py | 15 +---
>> src/py/examples/simple-capture.py | 21 ++----
>> src/py/examples/simple-continuous-capture.py | 21 ++----
>> src/py/libcamera/py_main.cpp | 62 ++++++++++++----
>> test/py/unittests.py | 76 +++++++-------------
>> 6 files changed, 93 insertions(+), 118 deletions(-)
>>
>> diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py
>> index 2701d937..6b6b678b 100755
>> --- a/src/py/cam/cam.py
>> +++ b/src/py/cam/cam.py
>> @@ -158,9 +158,7 @@ class CameraContext:
>>
>> print('Camera configuration adjusted')
>>
>> - r = self.camera.configure(camconfig)
>> - if r != 0:
>> - raise Exception('Configure failed')
>> + self.camera.configure(camconfig)
>>
>> self.stream_names = {}
>> self.streams = []
>> @@ -175,12 +173,7 @@ class CameraContext:
>> allocator = libcam.FrameBufferAllocator(self.camera)
>>
>> for stream in self.streams:
>> - ret = allocator.allocate(stream)
>> - if ret < 0:
>> - print('Cannot allocate buffers')
>> - exit(-1)
>> -
>> - allocated = len(allocator.buffers(stream))
>> + allocated = allocator.allocate(stream)
>>
>> print('{}-{}: Allocated {} buffers'.format(self.id, self.stream_names[stream], allocated))
>>
>> @@ -205,10 +198,7 @@ class CameraContext:
>> buffers = self.allocator.buffers(stream)
>> buffer = buffers[buf_num]
>>
>> - ret = request.add_buffer(stream, buffer)
>> - if ret < 0:
>> - print('Can not set buffer for request')
>> - exit(-1)
>> + request.add_buffer(stream, buffer)
>>
>> requests.append(request)
>>
>> diff --git a/src/py/examples/simple-cam.py b/src/py/examples/simple-cam.py
>> index b3e97ca7..1cd1019d 100755
>> --- a/src/py/examples/simple-cam.py
>> +++ b/src/py/examples/simple-cam.py
>> @@ -259,12 +259,7 @@ def main():
>> allocator = libcam.FrameBufferAllocator(camera)
>>
>> for cfg in config:
>> - ret = allocator.allocate(cfg.stream)
>> - if ret < 0:
>> - print('Can\'t allocate buffers')
>> - return -1
>> -
>> - allocated = len(allocator.buffers(cfg.stream))
>> + allocated = allocator.allocate(cfg.stream)
>> print(f'Allocated {allocated} buffers for stream')
>>
>> # --------------------------------------------------------------------
>> @@ -289,15 +284,9 @@ def main():
>> requests = []
>> for i in range(len(buffers)):
>> request = camera.create_request()
>> - if not request:
>> - print('Can\'t create request')
>> - return -1
>>
>> buffer = buffers[i]
>> - ret = request.add_buffer(stream, buffer)
>> - if ret < 0:
>> - print('Can\'t set buffer for request')
>> - return -1
>> + request.add_buffer(stream, buffer)
>>
>> # Controls can be added to a request on a per frame basis.
>> request.set_control(libcam.controls.Brightness, 0.5)
>> diff --git a/src/py/examples/simple-capture.py b/src/py/examples/simple-capture.py
>> index 5f93574f..4b85408f 100755
>> --- a/src/py/examples/simple-capture.py
>> +++ b/src/py/examples/simple-capture.py
>> @@ -43,8 +43,7 @@ def main():
>>
>> # Acquire the camera for our use
>>
>> - ret = cam.acquire()
>> - assert ret == 0
>> + cam.acquire()
>>
>> # Configure the camera
>>
>> @@ -60,8 +59,7 @@ def main():
>> w, h = [int(v) for v in args.size.split('x')]
>> stream_config.size = libcam.Size(w, h)
>>
>> - ret = cam.configure(cam_config)
>> - assert ret == 0
>> + cam.configure(cam_config)
>>
>> print(f'Capturing {TOTAL_FRAMES} frames with {stream_config}')
>>
>> @@ -83,15 +81,13 @@ def main():
>> req = cam.create_request(i)
>>
>> buffer = allocator.buffers(stream)[i]
>> - ret = req.add_buffer(stream, buffer)
>> - assert ret == 0
>> + req.add_buffer(stream, buffer)
>>
>> reqs.append(req)
>>
>> # Start the camera
>>
>> - ret = cam.start()
>> - assert ret == 0
>> + cam.start()
>>
>> # frames_queued and frames_done track the number of frames queued and done
>>
>> @@ -101,8 +97,7 @@ def main():
>> # Queue the requests to the camera
>>
>> for req in reqs:
>> - ret = cam.queue_request(req)
>> - assert ret == 0
>> + cam.queue_request(req)
>> frames_queued += 1
>>
>> # The main loop. Wait for the queued Requests to complete, process them,
>> @@ -155,13 +150,11 @@ def main():
>>
>> # Stop the camera
>>
>> - ret = cam.stop()
>> - assert ret == 0
>> + cam.stop()
>>
>> # Release the camera
>>
>> - ret = cam.release()
>> - assert ret == 0
>> + cam.release()
>>
>> return 0
>>
>> diff --git a/src/py/examples/simple-continuous-capture.py b/src/py/examples/simple-continuous-capture.py
>> index 26a8060b..e1cb931e 100755
>> --- a/src/py/examples/simple-continuous-capture.py
>> +++ b/src/py/examples/simple-continuous-capture.py
>> @@ -28,8 +28,7 @@ class CameraCaptureContext:
>>
>> # Acquire the camera for our use
>>
>> - ret = cam.acquire()
>> - assert ret == 0
>> + cam.acquire()
>>
>> # Configure the camera
>>
>> @@ -37,8 +36,7 @@ class CameraCaptureContext:
>>
>> stream_config = cam_config.at(0)
>>
>> - ret = cam.configure(cam_config)
>> - assert ret == 0
>> + cam.configure(cam_config)
>>
>> stream = stream_config.stream
>>
>> @@ -62,8 +60,7 @@ class CameraCaptureContext:
>> req = cam.create_request(idx)
>>
>> buffer = allocator.buffers(stream)[i]
>> - ret = req.add_buffer(stream, buffer)
>> - assert ret == 0
>> + req.add_buffer(stream, buffer)
>>
>> self.reqs.append(req)
>>
>> @@ -73,13 +70,11 @@ class CameraCaptureContext:
>> def uninit_camera(self):
>> # Stop the camera
>>
>> - ret = self.cam.stop()
>> - assert ret == 0
>> + self.cam.stop()
>>
>> # Release the camera
>>
>> - ret = self.cam.release()
>> - assert ret == 0
>> + self.cam.release()
>>
>>
>> # A container class for our state
>> @@ -145,8 +140,7 @@ class CaptureContext:
>>
>> for cam_ctx in self.camera_contexts:
>> for req in cam_ctx.reqs:
>> - ret = cam_ctx.cam.queue_request(req)
>> - assert ret == 0
>> + cam_ctx.cam.queue_request(req)
>>
>> # Use Selector to wait for events from the camera and from the keyboard
>>
>> @@ -177,8 +171,7 @@ def main():
>> # Start the cameras
>>
>> for cam_ctx in ctx.camera_contexts:
>> - ret = cam_ctx.cam.start()
>> - assert ret == 0
>> + cam_ctx.cam.start()
>>
>> ctx.capture()
>>
>> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
>> index e7d078b5..6cd99919 100644
>> --- a/src/py/libcamera/py_main.cpp
>> +++ b/src/py/libcamera/py_main.cpp
>> @@ -111,8 +111,19 @@ PYBIND11_MODULE(_libcamera, m)
>>
>> pyCamera
>> .def_property_readonly("id", &Camera::id)
>> - .def("acquire", &Camera::acquire)
>> - .def("release", &Camera::release)
>> + .def("acquire", [](Camera &self) {
>> + int ret = self.acquire();
>> + if (ret)
>> + throw std::system_error(-ret, std::generic_category(),
>> + "Failed to acquire camera");
>> + })
>> + .def("release", [](Camera &self) {
>> + int ret = self.release();
>> + /* \todo Should we ignore the error? */
>> + if (ret)
>> + throw std::system_error(-ret, std::generic_category(),
>> + "Failed to release camera");
>> + })
>> .def("start", [](Camera &self,
>> const std::unordered_map<const ControlId *, py::object> &controls) {
>> /* \todo What happens if someone calls start() multiple times? */
>> @@ -132,20 +143,20 @@ PYBIND11_MODULE(_libcamera, m)
>> int ret = self.start(&controlList);
>> if (ret) {
>> self.requestCompleted.disconnect();
>> - return ret;
>> + throw std::system_error(-ret, std::generic_category(),
>> + "Failed to start camera");
>> }
>> -
>> - return 0;
>> }, py::arg("controls") = std::unordered_map<const ControlId *, py::object>())
>>
>> .def("stop", [](Camera &self) {
>> int ret = self.stop();
>> - if (ret)
>> - return ret;
>>
>> self.requestCompleted.disconnect();
>>
>> - return 0;
>> + /* \todo Should we just ignore the error? */
>
> There previously was an assertion, so I think it's fine to keep
> checking for errors.
>
>> + if (ret)
>> + throw std::system_error(-ret, std::generic_category(),
>> + "Failed to start camera");
>
> s/start/stop
Indeed, thanks =).
Tomi
More information about the libcamera-devel
mailing list