[24/36] libcamera: global_configuration: Rename Configuration to Option
diff mbox series

Message ID 20260113000808.15395-25-laurent.pinchart@ideasonboard.com
State New
Headers show
Series
  • libcamera: Global configuration file improvements
Related show

Commit Message

Laurent Pinchart Jan. 13, 2026, 12:07 a.m. UTC 
The GlobalConfiguration::Configuration type represent a configuration
option. Rename it to Option to make this clearer. This shortens lines as
an added bonus.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/internal/global_configuration.h | 4 ++--
 src/libcamera/global_configuration.cpp            | 8 ++++----
 src/libcamera/pipeline/simple/simple.cpp          | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

Comments

Barnabás Pőcze Jan. 13, 2026, 4:45 p.m. UTC | #1 
2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta:
> The GlobalConfiguration::Configuration type represent a configuration
> option. Rename it to Option to make this clearer. This shortens lines as
> an added bonus.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---

To be honest I would remove the type alias altogether because I'm not
sure it adds sufficient value.

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


>   include/libcamera/internal/global_configuration.h | 4 ++--
>   src/libcamera/global_configuration.cpp            | 8 ++++----
>   src/libcamera/pipeline/simple/simple.cpp          | 2 +-
>   3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h
> index 5c907ee92bfe..2c0bfadb4676 100644
> --- a/include/libcamera/internal/global_configuration.h
> +++ b/include/libcamera/internal/global_configuration.h
> @@ -22,12 +22,12 @@ namespace libcamera {
>   class GlobalConfiguration
>   {
>   public:
> -	using Configuration = const ValueNode &;
> +	using Option = const ValueNode &;
>   
>   	GlobalConfiguration();
>   
>   	unsigned int version() const;
> -	Configuration configuration() const;
> +	Option configuration() const;
>   
>   	template<typename T>
>   	std::optional<T> option(
> diff --git a/src/libcamera/global_configuration.cpp b/src/libcamera/global_configuration.cpp
> index ee7d9c185b80..4d154c026e44 100644
> --- a/src/libcamera/global_configuration.cpp
> +++ b/src/libcamera/global_configuration.cpp
> @@ -56,8 +56,8 @@ LOG_DEFINE_CATEGORY(Configuration)
>    */
>   
>   /**
> - * \typedef GlobalConfiguration::Configuration
> - * \brief Type representing global libcamera configuration
> + * \typedef GlobalConfiguration::Option
> + * \brief Type representing a configuration option
>    *
>    * All code outside GlobalConfiguration must use this type declaration and not
>    * the underlying type.
> @@ -156,9 +156,9 @@ unsigned int GlobalConfiguration::version() const
>    * \note \a ValueNode type itself shouldn't be used in type declarations to
>    * avoid trouble if we decide to change the underlying data objects in future.
>    *
> - * \return The whole configuration section
> + * \return The top-level configuration option
>    */
> -GlobalConfiguration::Configuration GlobalConfiguration::configuration() const
> +GlobalConfiguration::Option GlobalConfiguration::configuration() const
>   {
>   	return (*configuration_)["configuration"];
>   }
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index b30b0a122e6e..bdc87d39ef0b 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -1880,7 +1880,7 @@ bool SimplePipelineHandler::matchDevice(std::shared_ptr<MediaDevice> media,
>   
>   	swIspEnabled_ = info.swIspEnabled;
>   	const GlobalConfiguration &configuration = cameraManager()->_d()->configuration();
> -	for (GlobalConfiguration::Configuration entry :
> +	for (GlobalConfiguration::Option entry :
>   	     configuration.configuration()["pipelines"]["simple"]["supported_devices"]
>   		     .asList()) {
>   		auto name = entry["driver"].get<std::string>();
Laurent Pinchart Jan. 13, 2026, 8:31 p.m. UTC | #2 
On Tue, Jan 13, 2026 at 05:45:31PM +0100, Barnabás Pőcze wrote:
> 2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta:
> > The GlobalConfiguration::Configuration type represent a configuration
> > option. Rename it to Option to make this clearer. This shortens lines as
> > an added bonus.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> 
> To be honest I would remove the type alias altogether because I'm not
> sure it adds sufficient value.

It was added to avoid depending directly on YamlObject, even if I wasn't
entirely convinced during review that it was a good idea. I would be
fine using ValueNode directly as it's now becoming a generic class
inside libcamera, not something tied to YAML parsing any more. I think
I'll keep this patch in v2, and maybe add a patch on top to remove the
alias. We can then decide if we want to merge it.

> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> 
> >   include/libcamera/internal/global_configuration.h | 4 ++--
> >   src/libcamera/global_configuration.cpp            | 8 ++++----
> >   src/libcamera/pipeline/simple/simple.cpp          | 2 +-
> >   3 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h
> > index 5c907ee92bfe..2c0bfadb4676 100644
> > --- a/include/libcamera/internal/global_configuration.h
> > +++ b/include/libcamera/internal/global_configuration.h
> > @@ -22,12 +22,12 @@ namespace libcamera {
> >   class GlobalConfiguration
> >   {
> >   public:
> > -	using Configuration = const ValueNode &;
> > +	using Option = const ValueNode &;
> >   
> >   	GlobalConfiguration();
> >   
> >   	unsigned int version() const;
> > -	Configuration configuration() const;
> > +	Option configuration() const;
> >   
> >   	template<typename T>
> >   	std::optional<T> option(
> > diff --git a/src/libcamera/global_configuration.cpp b/src/libcamera/global_configuration.cpp
> > index ee7d9c185b80..4d154c026e44 100644
> > --- a/src/libcamera/global_configuration.cpp
> > +++ b/src/libcamera/global_configuration.cpp
> > @@ -56,8 +56,8 @@ LOG_DEFINE_CATEGORY(Configuration)
> >    */
> >   
> >   /**
> > - * \typedef GlobalConfiguration::Configuration
> > - * \brief Type representing global libcamera configuration
> > + * \typedef GlobalConfiguration::Option
> > + * \brief Type representing a configuration option
> >    *
> >    * All code outside GlobalConfiguration must use this type declaration and not
> >    * the underlying type.
> > @@ -156,9 +156,9 @@ unsigned int GlobalConfiguration::version() const
> >    * \note \a ValueNode type itself shouldn't be used in type declarations to
> >    * avoid trouble if we decide to change the underlying data objects in future.
> >    *
> > - * \return The whole configuration section
> > + * \return The top-level configuration option
> >    */
> > -GlobalConfiguration::Configuration GlobalConfiguration::configuration() const
> > +GlobalConfiguration::Option GlobalConfiguration::configuration() const
> >   {
> >   	return (*configuration_)["configuration"];
> >   }
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index b30b0a122e6e..bdc87d39ef0b 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -1880,7 +1880,7 @@ bool SimplePipelineHandler::matchDevice(std::shared_ptr<MediaDevice> media,
> >   
> >   	swIspEnabled_ = info.swIspEnabled;
> >   	const GlobalConfiguration &configuration = cameraManager()->_d()->configuration();
> > -	for (GlobalConfiguration::Configuration entry :
> > +	for (GlobalConfiguration::Option entry :
> >   	     configuration.configuration()["pipelines"]["simple"]["supported_devices"]
> >   		     .asList()) {
> >   		auto name = entry["driver"].get<std::string>();

Patch
diff mbox series 

diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h
index 5c907ee92bfe..2c0bfadb4676 100644
--- a/include/libcamera/internal/global_configuration.h
+++ b/include/libcamera/internal/global_configuration.h
@@ -22,12 +22,12 @@  namespace libcamera {
 class GlobalConfiguration
 {
 public:
-	using Configuration = const ValueNode &;
+	using Option = const ValueNode &;
 
 	GlobalConfiguration();
 
 	unsigned int version() const;
-	Configuration configuration() const;
+	Option configuration() const;
 
 	template<typename T>
 	std::optional<T> option(
diff --git a/src/libcamera/global_configuration.cpp b/src/libcamera/global_configuration.cpp
index ee7d9c185b80..4d154c026e44 100644
--- a/src/libcamera/global_configuration.cpp
+++ b/src/libcamera/global_configuration.cpp
@@ -56,8 +56,8 @@  LOG_DEFINE_CATEGORY(Configuration)
  */
 
 /**
- * \typedef GlobalConfiguration::Configuration
- * \brief Type representing global libcamera configuration
+ * \typedef GlobalConfiguration::Option
+ * \brief Type representing a configuration option
  *
  * All code outside GlobalConfiguration must use this type declaration and not
  * the underlying type.
@@ -156,9 +156,9 @@  unsigned int GlobalConfiguration::version() const
  * \note \a ValueNode type itself shouldn't be used in type declarations to
  * avoid trouble if we decide to change the underlying data objects in future.
  *
- * \return The whole configuration section
+ * \return The top-level configuration option
  */
-GlobalConfiguration::Configuration GlobalConfiguration::configuration() const
+GlobalConfiguration::Option GlobalConfiguration::configuration() const
 {
 	return (*configuration_)["configuration"];
 }
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index b30b0a122e6e..bdc87d39ef0b 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -1880,7 +1880,7 @@  bool SimplePipelineHandler::matchDevice(std::shared_ptr<MediaDevice> media,
 
 	swIspEnabled_ = info.swIspEnabled;
 	const GlobalConfiguration &configuration = cameraManager()->_d()->configuration();
-	for (GlobalConfiguration::Configuration entry :
+	for (GlobalConfiguration::Option entry :
 	     configuration.configuration()["pipelines"]["simple"]["supported_devices"]
 		     .asList()) {
 		auto name = entry["driver"].get<std::string>();