[libcamera-devel] [PATCH v2 1/3] cam: Do not assume Location is available

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Mar 19, 2021 at 02:01:18PM +0100, Jacopo Mondi wrote:
> In preparation to register the Location property only if the firware
> interface provides it, do not assume it is available and build the
> camera name using the camera sensor model as a fallback.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/cam/main.cpp | 32 ++++++++++++++++++++------------
>  1 file changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index e01be63a7058..c087cdd97332 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -379,18 +379,26 @@ std::string const CamApp::cameraName(const Camera *camera)
>  	const ControlList &props = camera->properties();
>  	std::string name;
>  
> -	switch (props.get(properties::Location)) {
> -	case properties::CameraLocationFront:
> -		name = "Internal front camera";
> -		break;
> -	case properties::CameraLocationBack:
> -		name = "Internal back camera";
> -		break;
> -	case properties::CameraLocationExternal:
> -		name = "External camera";
> -		if (props.contains(properties::Model))
> -			name += " '" + props.get(properties::Model) + "'";
> -		break;
> +	if (props.contains(properties::Location)) {
> +		switch (props.get(properties::Location)) {
> +		case properties::CameraLocationFront:
> +			name = "Internal front camera";
> +			break;
> +		case properties::CameraLocationBack:
> +			name = "Internal back camera";
> +			break;
> +		case properties::CameraLocationExternal:
> +			name = "External camera";
> +			if (props.contains(properties::Model))
> +				name += " '" + props.get(properties::Model) + "'";
> +			break;
> +		}
> +	} else if (props.contains(properties::Model)) {
> +		/*
> +		 * If the camera location is not availble use the camera model
> +		 * to build the camera name.
> +		 */
> +		name = "'" + props.get(properties::Model) + "'";
>  	}
>  
>  	name += " (" + camera->id() + ")";

If none of the conditions above is true, there will be an extra space.
I initially thought utils::join() could be useful here, but that's not a
public API :-S This could be an option, I'm sure there are other equally
valid or better solutions.

diff --git a/src/cam/main.cpp b/src/cam/main.cpp
index e01be63a7058..3adfe2c34969 100644
--- a/src/cam/main.cpp
+++ b/src/cam/main.cpp
@@ -377,23 +377,34 @@ int CamApp::run()
 std::string const CamApp::cameraName(const Camera *camera)
 {
 	const ControlList &props = camera->properties();
+	bool addModel = true;
 	std::string name;

-	switch (props.get(properties::Location)) {
-	case properties::CameraLocationFront:
-		name = "Internal front camera";
-		break;
-	case properties::CameraLocationBack:
-		name = "Internal back camera";
-		break;
-	case properties::CameraLocationExternal:
-		name = "External camera";
-		if (props.contains(properties::Model))
-			name += " '" + props.get(properties::Model) + "'";
-		break;
+	/*
+	 * Construct the name from the camera location, model and ID. The model
+	 * is only used if the location isn't present or is set to External.
+	 */
+
+	if (props.contains(properties::Location)) {
+		switch (props.get(properties::Location)) {
+		case properties::CameraLocationFront:
+			name = "Internal front camera ";
+			addModel = false;
+			break;
+		case properties::CameraLocationBack:
+			name = "Internal back camera ";
+			addModel = false;
+			break;
+		case properties::CameraLocationExternal:
+			name = "External camera ";
+			break;
+		}
 	}

-	name += " (" + camera->id() + ")";
+	if (addModel && props.contains(properties::Model))
+		name += "'" + props.get(properties::Model) + "' ";
+
+	name += "(" + camera->id() + ")";

 	return name;
 }

It's a detail, and regardless of what option you pick,

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

-- 
Regards,

Laurent Pinchart