| Message ID | 20220701084521.31831-11-tomi.valkeinen@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
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@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 Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > }) > > .def("__str__", [](Camera &self) { > @@ -154,9 +165,20 @@ PYBIND11_MODULE(_libcamera, m) > > /* Keep the camera alive, as StreamConfiguration contains a Stream* */ > .def("generate_configuration", &Camera::generateConfiguration, py::keep_alive<0, 1>()) > - .def("configure", &Camera::configure) > + .def("configure", [](Camera &self, CameraConfiguration *config) { > + int ret = self.configure(config); > + if (ret) > + throw std::system_error(-ret, std::generic_category(), > + "Failed to configure camera"); > + }) > > - .def("create_request", &Camera::createRequest, py::arg("cookie") = 0) > + .def("create_request", [](Camera &self, uint64_t cookie) { > + std::unique_ptr<Request> req = self.createRequest(cookie); > + if (!req) > + throw std::system_error(ENOMEM, std::generic_category(), > + "Failed to create request"); > + return req; > + }, py::arg("cookie") = 0) > > .def("queue_request", [](Camera &self, Request *req) { > py::object py_req = py::cast(req); > @@ -169,10 +191,11 @@ PYBIND11_MODULE(_libcamera, m) > py_req.inc_ref(); > > int ret = self.queueRequest(req); > - if (ret) > + if (ret) { > py_req.dec_ref(); > - > - return ret; > + throw std::system_error(-ret, std::generic_category(), > + "Failed to queue request"); > + } > }) > > .def_property_readonly("streams", [](Camera &self) { > @@ -250,7 +273,13 @@ PYBIND11_MODULE(_libcamera, m) > > pyFrameBufferAllocator > .def(py::init<std::shared_ptr<Camera>>(), py::keep_alive<1, 2>()) > - .def("allocate", &FrameBufferAllocator::allocate) > + .def("allocate", [](FrameBufferAllocator &self, Stream *stream) { > + int ret = self.allocate(stream); > + if (ret < 0) > + throw std::system_error(-ret, std::generic_category(), > + "Failed to queue request"); > + return ret; > + }) > .def_property_readonly("allocated", &FrameBufferAllocator::allocated) > /* Create a list of FrameBuffers, where each FrameBuffer has a keep-alive to FrameBufferAllocator */ > .def("buffers", [](FrameBufferAllocator &self, Stream *stream) { > @@ -327,7 +356,10 @@ PYBIND11_MODULE(_libcamera, m) > pyRequest > /* \todo Fence is not supported, so we cannot expose addBuffer() directly */ > .def("add_buffer", [](Request &self, const Stream *stream, FrameBuffer *buffer) { > - return self.addBuffer(stream, buffer); > + int ret = self.addBuffer(stream, buffer); > + if (ret) > + throw std::system_error(-ret, std::generic_category(), > + "Failed to add buffer"); > }, py::keep_alive<1, 3>()) /* Request keeps Framebuffer alive */ > .def_property_readonly("status", &Request::status) > .def_property_readonly("buffers", &Request::buffers) > diff --git a/test/py/unittests.py b/test/py/unittests.py > index 6dea80fc..794e46be 100755 > --- a/test/py/unittests.py > +++ b/test/py/unittests.py > @@ -42,31 +42,26 @@ class SimpleTestMethods(BaseTestCase): > cam = cm.get('platform/vimc.0 Sensor B') > self.assertIsNotNone(cam) > > - ret = cam.acquire() > - self.assertZero(ret) > + cam.acquire() > > - ret = cam.release() > - self.assertZero(ret) > + cam.release() > > def test_double_acquire(self): > cm = libcam.CameraManager.singleton() > cam = cm.get('platform/vimc.0 Sensor B') > self.assertIsNotNone(cam) > > - ret = cam.acquire() > - self.assertZero(ret) > + cam.acquire() > > libcam.log_set_level('Camera', 'FATAL') > - ret = cam.acquire() > - self.assertEqual(ret, -errno.EBUSY) > + with self.assertRaises(RuntimeError): > + cam.acquire() > libcam.log_set_level('Camera', 'ERROR') > > - ret = cam.release() > - self.assertZero(ret) > + cam.release() > > - ret = cam.release() > - # I expected EBUSY, but looks like double release works fine > - self.assertZero(ret) > + # I expected exception here, but looks like double release works fine > + cam.release() > > > class CameraTesterBase(BaseTestCase): > @@ -80,11 +75,7 @@ class CameraTesterBase(BaseTestCase): > self.cm = None > self.skipTest('No vimc found') > > - ret = self.cam.acquire() > - if ret != 0: > - self.cam = None > - self.cm = None > - raise Exception('Failed to acquire camera') > + self.cam.acquire() > > self.wr_cam = weakref.ref(self.cam) > self.wr_cm = weakref.ref(self.cm) > @@ -93,9 +84,7 @@ class CameraTesterBase(BaseTestCase): > # If a test fails, the camera may be in running state. So always stop. > self.cam.stop() > > - ret = self.cam.release() > - if ret != 0: > - raise Exception('Failed to release camera') > + self.cam.release() > > self.cam = None > self.cm = None > @@ -115,8 +104,7 @@ class AllocatorTestMethods(CameraTesterBase): > streamconfig = camconfig.at(0) > wr_streamconfig = weakref.ref(streamconfig) > > - ret = cam.configure(camconfig) > - self.assertZero(ret) > + cam.configure(camconfig) > > stream = streamconfig.stream > wr_stream = weakref.ref(stream) > @@ -129,8 +117,8 @@ class AllocatorTestMethods(CameraTesterBase): > self.assertIsNotNone(wr_streamconfig()) > > allocator = libcam.FrameBufferAllocator(cam) > - ret = allocator.allocate(stream) > - self.assertTrue(ret > 0) > + num_bufs = allocator.allocate(stream) > + self.assertTrue(num_bufs > 0) > wr_allocator = weakref.ref(allocator) > > buffers = allocator.buffers(stream) > @@ -173,14 +161,13 @@ class SimpleCaptureMethods(CameraTesterBase): > self.assertIsNotNone(fmts) > fmts = None > > - ret = cam.configure(camconfig) > - self.assertZero(ret) > + cam.configure(camconfig) > > stream = streamconfig.stream > > allocator = libcam.FrameBufferAllocator(cam) > - ret = allocator.allocate(stream) > - self.assertTrue(ret > 0) > + num_bufs = allocator.allocate(stream) > + self.assertTrue(num_bufs > 0) > > num_bufs = len(allocator.buffers(stream)) > > @@ -190,19 +177,16 @@ class SimpleCaptureMethods(CameraTesterBase): > self.assertIsNotNone(req) > > buffer = allocator.buffers(stream)[i] > - ret = req.add_buffer(stream, buffer) > - self.assertZero(ret) > + req.add_buffer(stream, buffer) > > reqs.append(req) > > buffer = None > > - ret = cam.start() > - self.assertZero(ret) > + cam.start() > > for req in reqs: > - ret = cam.queue_request(req) > - self.assertZero(ret) > + cam.queue_request(req) > > reqs = None > gc.collect() > @@ -230,8 +214,7 @@ class SimpleCaptureMethods(CameraTesterBase): > reqs = None > gc.collect() > > - ret = cam.stop() > - self.assertZero(ret) > + cam.stop() > > def test_select(self): > cm = self.cm > @@ -245,14 +228,13 @@ class SimpleCaptureMethods(CameraTesterBase): > self.assertIsNotNone(fmts) > fmts = None > > - ret = cam.configure(camconfig) > - self.assertZero(ret) > + cam.configure(camconfig) > > stream = streamconfig.stream > > allocator = libcam.FrameBufferAllocator(cam) > - ret = allocator.allocate(stream) > - self.assertTrue(ret > 0) > + num_bufs = allocator.allocate(stream) > + self.assertTrue(num_bufs > 0) > > num_bufs = len(allocator.buffers(stream)) > > @@ -262,19 +244,16 @@ class SimpleCaptureMethods(CameraTesterBase): > self.assertIsNotNone(req) > > buffer = allocator.buffers(stream)[i] > - ret = req.add_buffer(stream, buffer) > - self.assertZero(ret) > + req.add_buffer(stream, buffer) > > reqs.append(req) > > buffer = None > > - ret = cam.start() > - self.assertZero(ret) > + cam.start() > > for req in reqs: > - ret = cam.queue_request(req) > - self.assertZero(ret) > + cam.queue_request(req) > > reqs = None > gc.collect() > @@ -303,8 +282,7 @@ class SimpleCaptureMethods(CameraTesterBase): > reqs = None > gc.collect() > > - ret = cam.stop() > - self.assertZero(ret) > + cam.stop() > > > # Recursively expand slist's objects into olist, using seen to track already > -- > 2.34.1 >
On Thu, Aug 18, 2022 at 04:36:06PM +0200, Jacopo Mondi wrote: > 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++ s/mimicing/mimicking/ > > 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@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? */ Should we ? It indicates an error in the caller, right ? > > + 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 > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > > }) > > > > .def("__str__", [](Camera &self) { > > @@ -154,9 +165,20 @@ PYBIND11_MODULE(_libcamera, m) > > > > /* Keep the camera alive, as StreamConfiguration contains a Stream* */ > > .def("generate_configuration", &Camera::generateConfiguration, py::keep_alive<0, 1>()) > > - .def("configure", &Camera::configure) > > + .def("configure", [](Camera &self, CameraConfiguration *config) { > > + int ret = self.configure(config); > > + if (ret) > > + throw std::system_error(-ret, std::generic_category(), > > + "Failed to configure camera"); > > + }) > > > > - .def("create_request", &Camera::createRequest, py::arg("cookie") = 0) > > + .def("create_request", [](Camera &self, uint64_t cookie) { > > + std::unique_ptr<Request> req = self.createRequest(cookie); > > + if (!req) > > + throw std::system_error(ENOMEM, std::generic_category(), > > + "Failed to create request"); > > + return req; > > + }, py::arg("cookie") = 0) > > > > .def("queue_request", [](Camera &self, Request *req) { > > py::object py_req = py::cast(req); > > @@ -169,10 +191,11 @@ PYBIND11_MODULE(_libcamera, m) > > py_req.inc_ref(); > > > > int ret = self.queueRequest(req); > > - if (ret) > > + if (ret) { > > py_req.dec_ref(); > > - > > - return ret; > > + throw std::system_error(-ret, std::generic_category(), > > + "Failed to queue request"); > > + } > > }) > > > > .def_property_readonly("streams", [](Camera &self) { > > @@ -250,7 +273,13 @@ PYBIND11_MODULE(_libcamera, m) > > > > pyFrameBufferAllocator > > .def(py::init<std::shared_ptr<Camera>>(), py::keep_alive<1, 2>()) > > - .def("allocate", &FrameBufferAllocator::allocate) > > + .def("allocate", [](FrameBufferAllocator &self, Stream *stream) { > > + int ret = self.allocate(stream); > > + if (ret < 0) Shouldn't 0 be also considered an error ? > > + throw std::system_error(-ret, std::generic_category(), > > + "Failed to queue request"); Bad copy & paste. > > + return ret; > > + }) > > .def_property_readonly("allocated", &FrameBufferAllocator::allocated) > > /* Create a list of FrameBuffers, where each FrameBuffer has a keep-alive to FrameBufferAllocator */ > > .def("buffers", [](FrameBufferAllocator &self, Stream *stream) { > > @@ -327,7 +356,10 @@ PYBIND11_MODULE(_libcamera, m) > > pyRequest > > /* \todo Fence is not supported, so we cannot expose addBuffer() directly */ > > .def("add_buffer", [](Request &self, const Stream *stream, FrameBuffer *buffer) { > > - return self.addBuffer(stream, buffer); > > + int ret = self.addBuffer(stream, buffer); Extra space after = > > + if (ret) > > + throw std::system_error(-ret, std::generic_category(), > > + "Failed to add buffer"); > > }, py::keep_alive<1, 3>()) /* Request keeps Framebuffer alive */ > > .def_property_readonly("status", &Request::status) > > .def_property_readonly("buffers", &Request::buffers) > > diff --git a/test/py/unittests.py b/test/py/unittests.py > > index 6dea80fc..794e46be 100755 > > --- a/test/py/unittests.py > > +++ b/test/py/unittests.py > > @@ -42,31 +42,26 @@ class SimpleTestMethods(BaseTestCase): > > cam = cm.get('platform/vimc.0 Sensor B') > > self.assertIsNotNone(cam) > > > > - ret = cam.acquire() > > - self.assertZero(ret) > > + cam.acquire() Will the test framework produce an error report that provide as much debugging information as the assert functions ? > > > > - ret = cam.release() > > - self.assertZero(ret) > > + cam.release() > > > > def test_double_acquire(self): > > cm = libcam.CameraManager.singleton() > > cam = cm.get('platform/vimc.0 Sensor B') > > self.assertIsNotNone(cam) > > > > - ret = cam.acquire() > > - self.assertZero(ret) > > + cam.acquire() > > > > libcam.log_set_level('Camera', 'FATAL') > > - ret = cam.acquire() > > - self.assertEqual(ret, -errno.EBUSY) > > + with self.assertRaises(RuntimeError): > > + cam.acquire() > > libcam.log_set_level('Camera', 'ERROR') > > > > - ret = cam.release() > > - self.assertZero(ret) > > + cam.release() > > > > - ret = cam.release() > > - # I expected EBUSY, but looks like double release works fine > > - self.assertZero(ret) > > + # I expected exception here, but looks like double release works fine > > + cam.release() > > > > > > class CameraTesterBase(BaseTestCase): > > @@ -80,11 +75,7 @@ class CameraTesterBase(BaseTestCase): > > self.cm = None > > self.skipTest('No vimc found') > > > > - ret = self.cam.acquire() > > - if ret != 0: > > - self.cam = None > > - self.cm = None > > - raise Exception('Failed to acquire camera') > > + self.cam.acquire() > > > > self.wr_cam = weakref.ref(self.cam) > > self.wr_cm = weakref.ref(self.cm) > > @@ -93,9 +84,7 @@ class CameraTesterBase(BaseTestCase): > > # If a test fails, the camera may be in running state. So always stop. > > self.cam.stop() > > > > - ret = self.cam.release() > > - if ret != 0: > > - raise Exception('Failed to release camera') > > + self.cam.release() > > > > self.cam = None > > self.cm = None > > @@ -115,8 +104,7 @@ class AllocatorTestMethods(CameraTesterBase): > > streamconfig = camconfig.at(0) > > wr_streamconfig = weakref.ref(streamconfig) > > > > - ret = cam.configure(camconfig) > > - self.assertZero(ret) > > + cam.configure(camconfig) > > > > stream = streamconfig.stream > > wr_stream = weakref.ref(stream) > > @@ -129,8 +117,8 @@ class AllocatorTestMethods(CameraTesterBase): > > self.assertIsNotNone(wr_streamconfig()) > > > > allocator = libcam.FrameBufferAllocator(cam) > > - ret = allocator.allocate(stream) > > - self.assertTrue(ret > 0) > > + num_bufs = allocator.allocate(stream) > > + self.assertTrue(num_bufs > 0) > > wr_allocator = weakref.ref(allocator) > > > > buffers = allocator.buffers(stream) > > @@ -173,14 +161,13 @@ class SimpleCaptureMethods(CameraTesterBase): > > self.assertIsNotNone(fmts) > > fmts = None > > > > - ret = cam.configure(camconfig) > > - self.assertZero(ret) > > + cam.configure(camconfig) > > > > stream = streamconfig.stream > > > > allocator = libcam.FrameBufferAllocator(cam) > > - ret = allocator.allocate(stream) > > - self.assertTrue(ret > 0) > > + num_bufs = allocator.allocate(stream) > > + self.assertTrue(num_bufs > 0) > > > > num_bufs = len(allocator.buffers(stream)) > > > > @@ -190,19 +177,16 @@ class SimpleCaptureMethods(CameraTesterBase): > > self.assertIsNotNone(req) > > > > buffer = allocator.buffers(stream)[i] > > - ret = req.add_buffer(stream, buffer) > > - self.assertZero(ret) > > + req.add_buffer(stream, buffer) > > > > reqs.append(req) > > > > buffer = None > > > > - ret = cam.start() > > - self.assertZero(ret) > > + cam.start() > > > > for req in reqs: > > - ret = cam.queue_request(req) > > - self.assertZero(ret) > > + cam.queue_request(req) > > > > reqs = None > > gc.collect() > > @@ -230,8 +214,7 @@ class SimpleCaptureMethods(CameraTesterBase): > > reqs = None > > gc.collect() > > > > - ret = cam.stop() > > - self.assertZero(ret) > > + cam.stop() > > > > def test_select(self): > > cm = self.cm > > @@ -245,14 +228,13 @@ class SimpleCaptureMethods(CameraTesterBase): > > self.assertIsNotNone(fmts) > > fmts = None > > > > - ret = cam.configure(camconfig) > > - self.assertZero(ret) > > + cam.configure(camconfig) > > > > stream = streamconfig.stream > > > > allocator = libcam.FrameBufferAllocator(cam) > > - ret = allocator.allocate(stream) > > - self.assertTrue(ret > 0) > > + num_bufs = allocator.allocate(stream) > > + self.assertTrue(num_bufs > 0) > > > > num_bufs = len(allocator.buffers(stream)) > > > > @@ -262,19 +244,16 @@ class SimpleCaptureMethods(CameraTesterBase): > > self.assertIsNotNone(req) > > > > buffer = allocator.buffers(stream)[i] > > - ret = req.add_buffer(stream, buffer) > > - self.assertZero(ret) > > + req.add_buffer(stream, buffer) > > > > reqs.append(req) > > > > buffer = None > > > > - ret = cam.start() > > - self.assertZero(ret) > > + cam.start() > > > > for req in reqs: > > - ret = cam.queue_request(req) > > - self.assertZero(ret) > > + cam.queue_request(req) > > > > reqs = None > > gc.collect() > > @@ -303,8 +282,7 @@ class SimpleCaptureMethods(CameraTesterBase): > > reqs = None > > gc.collect() > > > > - ret = cam.stop() > > - self.assertZero(ret) > > + cam.stop() > > > > > > # Recursively expand slist's objects into olist, using seen to track already
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@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
On 18/08/2022 23:56, Laurent Pinchart wrote: > On Thu, Aug 18, 2022 at 04:36:06PM +0200, Jacopo Mondi wrote: >> 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++ > > s/mimicing/mimicking/ > >>> 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@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? */ > > Should we ? It indicates an error in the caller, right ? I find errors in stop/release/free style functions a bit difficult to understand. What does it mean? What is the caller supposed to do? Always ignore? Or always abort()? However, here, in the bindings, I think it makes sense to throw the exception if the C++ API gives an error, whether the C++ API's behavior makes sense or not. We're trying to expose the C++ API as directly as possible. So I'll drop these todo comments. >>> + 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 >> >> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> >> >>> }) >>> >>> .def("__str__", [](Camera &self) { >>> @@ -154,9 +165,20 @@ PYBIND11_MODULE(_libcamera, m) >>> >>> /* Keep the camera alive, as StreamConfiguration contains a Stream* */ >>> .def("generate_configuration", &Camera::generateConfiguration, py::keep_alive<0, 1>()) >>> - .def("configure", &Camera::configure) >>> + .def("configure", [](Camera &self, CameraConfiguration *config) { >>> + int ret = self.configure(config); >>> + if (ret) >>> + throw std::system_error(-ret, std::generic_category(), >>> + "Failed to configure camera"); >>> + }) >>> >>> - .def("create_request", &Camera::createRequest, py::arg("cookie") = 0) >>> + .def("create_request", [](Camera &self, uint64_t cookie) { >>> + std::unique_ptr<Request> req = self.createRequest(cookie); >>> + if (!req) >>> + throw std::system_error(ENOMEM, std::generic_category(), >>> + "Failed to create request"); >>> + return req; >>> + }, py::arg("cookie") = 0) >>> >>> .def("queue_request", [](Camera &self, Request *req) { >>> py::object py_req = py::cast(req); >>> @@ -169,10 +191,11 @@ PYBIND11_MODULE(_libcamera, m) >>> py_req.inc_ref(); >>> >>> int ret = self.queueRequest(req); >>> - if (ret) >>> + if (ret) { >>> py_req.dec_ref(); >>> - >>> - return ret; >>> + throw std::system_error(-ret, std::generic_category(), >>> + "Failed to queue request"); >>> + } >>> }) >>> >>> .def_property_readonly("streams", [](Camera &self) { >>> @@ -250,7 +273,13 @@ PYBIND11_MODULE(_libcamera, m) >>> >>> pyFrameBufferAllocator >>> .def(py::init<std::shared_ptr<Camera>>(), py::keep_alive<1, 2>()) >>> - .def("allocate", &FrameBufferAllocator::allocate) >>> + .def("allocate", [](FrameBufferAllocator &self, Stream *stream) { >>> + int ret = self.allocate(stream); >>> + if (ret < 0) > > Shouldn't 0 be also considered an error ? The function documentation says "The number of allocated buffers on success or a negative error code otherwise". So 0 doesn't seem to be an error. >>> + throw std::system_error(-ret, std::generic_category(), >>> + "Failed to queue request"); > > Bad copy & paste. Indeed. >>> + return ret; >>> + }) >>> .def_property_readonly("allocated", &FrameBufferAllocator::allocated) >>> /* Create a list of FrameBuffers, where each FrameBuffer has a keep-alive to FrameBufferAllocator */ >>> .def("buffers", [](FrameBufferAllocator &self, Stream *stream) { >>> @@ -327,7 +356,10 @@ PYBIND11_MODULE(_libcamera, m) >>> pyRequest >>> /* \todo Fence is not supported, so we cannot expose addBuffer() directly */ >>> .def("add_buffer", [](Request &self, const Stream *stream, FrameBuffer *buffer) { >>> - return self.addBuffer(stream, buffer); >>> + int ret = self.addBuffer(stream, buffer); > > Extra space after = Yep. >>> + if (ret) >>> + throw std::system_error(-ret, std::generic_category(), >>> + "Failed to add buffer"); >>> }, py::keep_alive<1, 3>()) /* Request keeps Framebuffer alive */ >>> .def_property_readonly("status", &Request::status) >>> .def_property_readonly("buffers", &Request::buffers) >>> diff --git a/test/py/unittests.py b/test/py/unittests.py >>> index 6dea80fc..794e46be 100755 >>> --- a/test/py/unittests.py >>> +++ b/test/py/unittests.py >>> @@ -42,31 +42,26 @@ class SimpleTestMethods(BaseTestCase): >>> cam = cm.get('platform/vimc.0 Sensor B') >>> self.assertIsNotNone(cam) >>> >>> - ret = cam.acquire() >>> - self.assertZero(ret) >>> + cam.acquire() > > Will the test framework produce an error report that provide as much > debugging information as the assert functions ? Yes. Although those two cases are slightly different: a failing test-assert results in test failure, and an exception results in test error. The end result is the same. Tomi
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? */ + if (ret) + throw std::system_error(-ret, std::generic_category(), + "Failed to start camera"); }) .def("__str__", [](Camera &self) { @@ -154,9 +165,20 @@ PYBIND11_MODULE(_libcamera, m) /* Keep the camera alive, as StreamConfiguration contains a Stream* */ .def("generate_configuration", &Camera::generateConfiguration, py::keep_alive<0, 1>()) - .def("configure", &Camera::configure) + .def("configure", [](Camera &self, CameraConfiguration *config) { + int ret = self.configure(config); + if (ret) + throw std::system_error(-ret, std::generic_category(), + "Failed to configure camera"); + }) - .def("create_request", &Camera::createRequest, py::arg("cookie") = 0) + .def("create_request", [](Camera &self, uint64_t cookie) { + std::unique_ptr<Request> req = self.createRequest(cookie); + if (!req) + throw std::system_error(ENOMEM, std::generic_category(), + "Failed to create request"); + return req; + }, py::arg("cookie") = 0) .def("queue_request", [](Camera &self, Request *req) { py::object py_req = py::cast(req); @@ -169,10 +191,11 @@ PYBIND11_MODULE(_libcamera, m) py_req.inc_ref(); int ret = self.queueRequest(req); - if (ret) + if (ret) { py_req.dec_ref(); - - return ret; + throw std::system_error(-ret, std::generic_category(), + "Failed to queue request"); + } }) .def_property_readonly("streams", [](Camera &self) { @@ -250,7 +273,13 @@ PYBIND11_MODULE(_libcamera, m) pyFrameBufferAllocator .def(py::init<std::shared_ptr<Camera>>(), py::keep_alive<1, 2>()) - .def("allocate", &FrameBufferAllocator::allocate) + .def("allocate", [](FrameBufferAllocator &self, Stream *stream) { + int ret = self.allocate(stream); + if (ret < 0) + throw std::system_error(-ret, std::generic_category(), + "Failed to queue request"); + return ret; + }) .def_property_readonly("allocated", &FrameBufferAllocator::allocated) /* Create a list of FrameBuffers, where each FrameBuffer has a keep-alive to FrameBufferAllocator */ .def("buffers", [](FrameBufferAllocator &self, Stream *stream) { @@ -327,7 +356,10 @@ PYBIND11_MODULE(_libcamera, m) pyRequest /* \todo Fence is not supported, so we cannot expose addBuffer() directly */ .def("add_buffer", [](Request &self, const Stream *stream, FrameBuffer *buffer) { - return self.addBuffer(stream, buffer); + int ret = self.addBuffer(stream, buffer); + if (ret) + throw std::system_error(-ret, std::generic_category(), + "Failed to add buffer"); }, py::keep_alive<1, 3>()) /* Request keeps Framebuffer alive */ .def_property_readonly("status", &Request::status) .def_property_readonly("buffers", &Request::buffers) diff --git a/test/py/unittests.py b/test/py/unittests.py index 6dea80fc..794e46be 100755 --- a/test/py/unittests.py +++ b/test/py/unittests.py @@ -42,31 +42,26 @@ class SimpleTestMethods(BaseTestCase): cam = cm.get('platform/vimc.0 Sensor B') self.assertIsNotNone(cam) - ret = cam.acquire() - self.assertZero(ret) + cam.acquire() - ret = cam.release() - self.assertZero(ret) + cam.release() def test_double_acquire(self): cm = libcam.CameraManager.singleton() cam = cm.get('platform/vimc.0 Sensor B') self.assertIsNotNone(cam) - ret = cam.acquire() - self.assertZero(ret) + cam.acquire() libcam.log_set_level('Camera', 'FATAL') - ret = cam.acquire() - self.assertEqual(ret, -errno.EBUSY) + with self.assertRaises(RuntimeError): + cam.acquire() libcam.log_set_level('Camera', 'ERROR') - ret = cam.release() - self.assertZero(ret) + cam.release() - ret = cam.release() - # I expected EBUSY, but looks like double release works fine - self.assertZero(ret) + # I expected exception here, but looks like double release works fine + cam.release() class CameraTesterBase(BaseTestCase): @@ -80,11 +75,7 @@ class CameraTesterBase(BaseTestCase): self.cm = None self.skipTest('No vimc found') - ret = self.cam.acquire() - if ret != 0: - self.cam = None - self.cm = None - raise Exception('Failed to acquire camera') + self.cam.acquire() self.wr_cam = weakref.ref(self.cam) self.wr_cm = weakref.ref(self.cm) @@ -93,9 +84,7 @@ class CameraTesterBase(BaseTestCase): # If a test fails, the camera may be in running state. So always stop. self.cam.stop() - ret = self.cam.release() - if ret != 0: - raise Exception('Failed to release camera') + self.cam.release() self.cam = None self.cm = None @@ -115,8 +104,7 @@ class AllocatorTestMethods(CameraTesterBase): streamconfig = camconfig.at(0) wr_streamconfig = weakref.ref(streamconfig) - ret = cam.configure(camconfig) - self.assertZero(ret) + cam.configure(camconfig) stream = streamconfig.stream wr_stream = weakref.ref(stream) @@ -129,8 +117,8 @@ class AllocatorTestMethods(CameraTesterBase): self.assertIsNotNone(wr_streamconfig()) allocator = libcam.FrameBufferAllocator(cam) - ret = allocator.allocate(stream) - self.assertTrue(ret > 0) + num_bufs = allocator.allocate(stream) + self.assertTrue(num_bufs > 0) wr_allocator = weakref.ref(allocator) buffers = allocator.buffers(stream) @@ -173,14 +161,13 @@ class SimpleCaptureMethods(CameraTesterBase): self.assertIsNotNone(fmts) fmts = None - ret = cam.configure(camconfig) - self.assertZero(ret) + cam.configure(camconfig) stream = streamconfig.stream allocator = libcam.FrameBufferAllocator(cam) - ret = allocator.allocate(stream) - self.assertTrue(ret > 0) + num_bufs = allocator.allocate(stream) + self.assertTrue(num_bufs > 0) num_bufs = len(allocator.buffers(stream)) @@ -190,19 +177,16 @@ class SimpleCaptureMethods(CameraTesterBase): self.assertIsNotNone(req) buffer = allocator.buffers(stream)[i] - ret = req.add_buffer(stream, buffer) - self.assertZero(ret) + req.add_buffer(stream, buffer) reqs.append(req) buffer = None - ret = cam.start() - self.assertZero(ret) + cam.start() for req in reqs: - ret = cam.queue_request(req) - self.assertZero(ret) + cam.queue_request(req) reqs = None gc.collect() @@ -230,8 +214,7 @@ class SimpleCaptureMethods(CameraTesterBase): reqs = None gc.collect() - ret = cam.stop() - self.assertZero(ret) + cam.stop() def test_select(self): cm = self.cm @@ -245,14 +228,13 @@ class SimpleCaptureMethods(CameraTesterBase): self.assertIsNotNone(fmts) fmts = None - ret = cam.configure(camconfig) - self.assertZero(ret) + cam.configure(camconfig) stream = streamconfig.stream allocator = libcam.FrameBufferAllocator(cam) - ret = allocator.allocate(stream) - self.assertTrue(ret > 0) + num_bufs = allocator.allocate(stream) + self.assertTrue(num_bufs > 0) num_bufs = len(allocator.buffers(stream)) @@ -262,19 +244,16 @@ class SimpleCaptureMethods(CameraTesterBase): self.assertIsNotNone(req) buffer = allocator.buffers(stream)[i] - ret = req.add_buffer(stream, buffer) - self.assertZero(ret) + req.add_buffer(stream, buffer) reqs.append(req) buffer = None - ret = cam.start() - self.assertZero(ret) + cam.start() for req in reqs: - ret = cam.queue_request(req) - self.assertZero(ret) + cam.queue_request(req) reqs = None gc.collect() @@ -303,8 +282,7 @@ class SimpleCaptureMethods(CameraTesterBase): reqs = None gc.collect() - ret = cam.stop() - self.assertZero(ret) + cam.stop() # Recursively expand slist's objects into olist, using seen to track already
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@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(-)