[libcamera-devel] [PATCH] libcamera: controls: Extend docs how to identify controls from ControlList

[Marian Cichy]

On 3/19/21 6:03 PM, Sebastian Fricke wrote:
> Hey Marian,
>
> Thank you for the patch.
>
> On 19.03.2021 17:40, Marian Cichy wrote:
>> Informations how to identify Controls from a ControlList is quite
>
> s/Informations how/Informations on how/
> s/is quite/are quite/
>
>> scattered around the documentation and not clear if one reads about the
>
> s/and not clear/and it is not clear/
>
>> ControlList. Referring to ControlId and ControlIdMap right in the
>> detailed description is probably very helpful.
>
> agreed :)
>
>>
>> Signed-off-by: Marian Cichy <m.cichy at pengutronix.de>
>> ---
>> src/libcamera/controls.cpp | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
>> index c58ed394..b5253f83 100644
>> --- a/src/libcamera/controls.cpp
>> +++ b/src/libcamera/controls.cpp
>> @@ -805,6 +805,10 @@ ControlList::ControlList()
>>  * For ControlList containing libcamera controls, a global map of all 
>> libcamera
>>  * controls is provided by controls::controls and can be used as the 
>> \a idmap
>>  * argument.
>> + *
>> + * To identify a Control from the ControlList, one needs to find the 
>> ControlId
>> + * from the numerical control id saved in this list. A global 
>> ControlIdMap
>> + * of all libcamera controls is provided by controls::controls.
>
> This sentence seems to be very similar to the sentence two rows above.
> How about:
> ```
>  * For ControlList containing libcamera controls, a global map of all 
> libcamera
>  * controls is provided by controls::controls and can be used as the 
> \a idmap
>  * argument.
>  * In order to identify a single control from the ControlList, one 
> needs to find
>  * the ControlId using the contains method in the numerical control 
> IDs of that
>  * map.
> ```
>

Not sure, why you would recommend contains() here, what are you trying 
to achieve? contains() won't give me any information about the mapped ID 
in any direction, right?

As an example, I'd write a snippet to merely iterate over all items in 
ControlList and use each numerical ID with controls::controls.at(). This 
was for me my first approach when I wanted to print the names of all my 
controls.

Regards,
Marian

> I feel like when we are at it, just mention how you are supposed to do
> it.
>
>>  */
>> ControlList::ControlList(const ControlIdMap &idmap, ControlValidator 
>> *validator)
>>     : validator_(validator), idmap_(&idmap), infoMap_(nullptr)
>> -- 
>> 2.29.2
>
> Greetings,
> Sebastian
>>
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel at lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
>