py: Use exceptions instead of returning error codes

We have multiple methods which return an error code, mimicking 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>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

src/py/cam/cam.py

@@ -158,9 +158,7 @@ class CameraContext:
 
             print('Camera configuration adjusted')
 
-        r = self.camera.configure(camconfig)
+        self.camera.configure(camconfig)
-        if r != 0:
-            raise Exception('Configure failed')
 
         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)
+            allocated = allocator.allocate(stream)
-            if ret < 0:
-                print('Cannot allocate buffers')
-                exit(-1)
-
-            allocated = len(allocator.buffers(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)
+                request.add_buffer(stream, buffer)
-                if ret < 0:
-                    print('Can not set buffer for request')
-                    exit(-1)
 
             requests.append(request)
 

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)
+        allocated = allocator.allocate(cfg.stream)
-        if ret < 0:
-            print('Can\'t allocate buffers')
-            return -1
-
-        allocated = len(allocator.buffers(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)
+        request.add_buffer(stream, buffer)
-        if ret < 0:
-            print('Can\'t set buffer for request')
-            return -1
 
         # Controls can be added to a request on a per frame basis.
         request.set_control(libcam.controls.Brightness, 0.5)

src/py/examples/simple-capture.py

@@ -43,8 +43,7 @@ def main():
 
     # Acquire the camera for our use
 
-    ret = cam.acquire()
+    cam.acquire()
-    assert ret == 0
 
     # 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)
+    cam.configure(cam_config)
-    assert ret == 0
 
     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)
+        req.add_buffer(stream, buffer)
-        assert ret == 0
 
         reqs.append(req)
 
     # Start the camera
 
-    ret = cam.start()
+    cam.start()
-    assert ret == 0
 
     # 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)
+        cam.queue_request(req)
-        assert ret == 0
         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()
+    cam.stop()
-    assert ret == 0
 
     # Release the camera
 
-    ret = cam.release()
+    cam.release()
-    assert ret == 0
 
     return 0
 

src/py/examples/simple-continuous-capture.py

@@ -28,8 +28,7 @@ class CameraCaptureContext:
 
         # Acquire the camera for our use
 
-        ret = cam.acquire()
+        cam.acquire()
-        assert ret == 0
 
         # Configure the camera
 
@@ -37,8 +36,7 @@ class CameraCaptureContext:
 
         stream_config = cam_config.at(0)
 
-        ret = cam.configure(cam_config)
+        cam.configure(cam_config)
-        assert ret == 0
 
         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)
+            req.add_buffer(stream, buffer)
-            assert ret == 0
 
             self.reqs.append(req)
 
@@ -73,13 +70,11 @@ class CameraCaptureContext:
     def uninit_camera(self):
         # Stop the camera
 
-        ret = self.cam.stop()
+        self.cam.stop()
-        assert ret == 0
 
         # Release the camera
 
-        ret = self.cam.release()
+        self.cam.release()
-        assert ret == 0
 
 
 # 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)
+                cam_ctx.cam.queue_request(req)
-                assert ret == 0
 
         # 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()
+        cam_ctx.cam.start()
-        assert ret == 0
 
     ctx.capture()
 

src/py/libcamera/py_main.cpp

@@ -112,8 +112,18 @@ PYBIND11_MODULE(_libcamera, m)
 
 	pyCamera
 		.def_property_readonly("id", &Camera::id)
-		.def("acquire", &Camera::acquire)
+		.def("acquire", [](Camera &self) {
-		.def("release", &Camera::release)
+			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();
+			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? */
@@ -133,20 +143,19 @@ 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;
+			if (ret)
+				throw std::system_error(-ret, std::generic_category(),
+							"Failed to stop camera");
 		})
 
 		.def("__str__", [](Camera &self) {
@@ -155,9 +164,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);
@@ -170,10 +190,11 @@ PYBIND11_MODULE(_libcamera, m)
 			py_req.inc_ref();
 
 			int ret = self.queueRequest(req);
-			if (ret)
+			if (ret) {
 				py_req.dec_ref();
-
+				throw std::system_error(-ret, std::generic_category(),
-			return ret;
+							"Failed to queue request");
+			}
 		})
 
 		.def_property_readonly("streams", [](Camera &self) {
@@ -251,7 +272,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 allocate buffers");
+			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) {
@@ -328,7 +355,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)

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()
+        cam.acquire()
-        self.assertZero(ret)
 
-        ret = cam.release()
+        cam.release()
-        self.assertZero(ret)
 
     def test_double_acquire(self):
         cm = libcam.CameraManager.singleton()
         cam = cm.get('platform/vimc.0 Sensor B')
         self.assertIsNotNone(cam)
 
-        ret = cam.acquire()
+        cam.acquire()
-        self.assertZero(ret)
 
         libcam.log_set_level('Camera', 'FATAL')
-        ret = cam.acquire()
+        with self.assertRaises(RuntimeError):
-        self.assertEqual(ret, -errno.EBUSY)
+            cam.acquire()
         libcam.log_set_level('Camera', 'ERROR')
 
-        ret = cam.release()
+        cam.release()
-        self.assertZero(ret)
 
-        ret = cam.release()
+        # I expected exception here, but looks like double release works fine
-        # I expected EBUSY, but looks like double release works fine
+        cam.release()
-        self.assertZero(ret)
 
     def test_version(self):
         cm = libcam.CameraManager.singleton()
@@ -84,11 +79,7 @@ class CameraTesterBase(BaseTestCase):
             self.cm = None
             self.skipTest('No vimc found')
 
-        ret = self.cam.acquire()
+        self.cam.acquire()
-        if ret != 0:
-            self.cam = None
-            self.cm = None
-            raise Exception('Failed to acquire camera')
 
         self.wr_cam = weakref.ref(self.cam)
         self.wr_cm = weakref.ref(self.cm)
@@ -97,9 +88,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()
+        self.cam.release()
-        if ret != 0:
-            raise Exception('Failed to release camera')
 
         self.cam = None
         self.cm = None
@@ -119,8 +108,7 @@ class AllocatorTestMethods(CameraTesterBase):
         streamconfig = camconfig.at(0)
         wr_streamconfig = weakref.ref(streamconfig)
 
-        ret = cam.configure(camconfig)
+        cam.configure(camconfig)
-        self.assertZero(ret)
 
         stream = streamconfig.stream
         wr_stream = weakref.ref(stream)
@@ -133,8 +121,8 @@ class AllocatorTestMethods(CameraTesterBase):
         self.assertIsNotNone(wr_streamconfig())
 
         allocator = libcam.FrameBufferAllocator(cam)
-        ret = allocator.allocate(stream)
+        num_bufs = allocator.allocate(stream)
-        self.assertTrue(ret > 0)
+        self.assertTrue(num_bufs > 0)
         wr_allocator = weakref.ref(allocator)
 
         buffers = allocator.buffers(stream)
@@ -177,14 +165,13 @@ class SimpleCaptureMethods(CameraTesterBase):
         self.assertIsNotNone(fmts)
         fmts = None
 
-        ret = cam.configure(camconfig)
+        cam.configure(camconfig)
-        self.assertZero(ret)
 
         stream = streamconfig.stream
 
         allocator = libcam.FrameBufferAllocator(cam)
-        ret = allocator.allocate(stream)
+        num_bufs = allocator.allocate(stream)
-        self.assertTrue(ret > 0)
+        self.assertTrue(num_bufs > 0)
 
         num_bufs = len(allocator.buffers(stream))
 
@@ -194,19 +181,16 @@ class SimpleCaptureMethods(CameraTesterBase):
             self.assertIsNotNone(req)
 
             buffer = allocator.buffers(stream)[i]
-            ret = req.add_buffer(stream, buffer)
+            req.add_buffer(stream, buffer)
-            self.assertZero(ret)
 
             reqs.append(req)
 
         buffer = None
 
-        ret = cam.start()
+        cam.start()
-        self.assertZero(ret)
 
         for req in reqs:
-            ret = cam.queue_request(req)
+            cam.queue_request(req)
-            self.assertZero(ret)
 
         reqs = None
         gc.collect()
@@ -234,8 +218,7 @@ class SimpleCaptureMethods(CameraTesterBase):
         reqs = None
         gc.collect()
 
-        ret = cam.stop()
+        cam.stop()
-        self.assertZero(ret)
 
     def test_select(self):
         cm = self.cm
@@ -249,14 +232,13 @@ class SimpleCaptureMethods(CameraTesterBase):
         self.assertIsNotNone(fmts)
         fmts = None
 
-        ret = cam.configure(camconfig)
+        cam.configure(camconfig)
-        self.assertZero(ret)
 
         stream = streamconfig.stream
 
         allocator = libcam.FrameBufferAllocator(cam)
-        ret = allocator.allocate(stream)
+        num_bufs = allocator.allocate(stream)
-        self.assertTrue(ret > 0)
+        self.assertTrue(num_bufs > 0)
 
         num_bufs = len(allocator.buffers(stream))
 
@@ -266,19 +248,16 @@ class SimpleCaptureMethods(CameraTesterBase):
             self.assertIsNotNone(req)
 
             buffer = allocator.buffers(stream)[i]
-            ret = req.add_buffer(stream, buffer)
+            req.add_buffer(stream, buffer)
-            self.assertZero(ret)
 
             reqs.append(req)
 
         buffer = None
 
-        ret = cam.start()
+        cam.start()
-        self.assertZero(ret)
 
         for req in reqs:
-            ret = cam.queue_request(req)
+            cam.queue_request(req)
-            self.assertZero(ret)
 
         reqs = None
         gc.collect()
@@ -307,8 +286,7 @@ class SimpleCaptureMethods(CameraTesterBase):
         reqs = None
         gc.collect()
 
-        ret = cam.stop()
+        cam.stop()
-        self.assertZero(ret)
 
 
 # Recursively expand slist's objects into olist, using seen to track already