[libcamera-devel] [PATCH v2 31/32] test: controls: control_value: Expand test to cover array controls

[Laurent Pinchart]

Hi Kieran,

On Fri, Mar 06, 2020 at 04:25:01PM +0000, Kieran Bingham wrote:
> On 06/03/2020 16:00, Laurent Pinchart wrote:
> > Add tests to ControlValueTest to cover array controls of all supported
> > types.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> One discussion item below though:
> 
> > ---
> > Changes since v1:
> > 
> > - Renamed int8s to bytes
> > ---
> >  test/controls/control_value.cpp | 101 ++++++++++++++++++++++++++++++++
> >  1 file changed, 101 insertions(+)
> > 
> > diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp
> > index 37f415302ff6..fd28bfe4604a 100644
> > --- a/test/controls/control_value.cpp
> > +++ b/test/controls/control_value.cpp
> > @@ -5,6 +5,7 @@
> >   * control_value.cpp - ControlValue tests
> >   */
> >  
> > +#include <algorithm>
> >  #include <iostream>
> >  
> >  #include <libcamera/controls.h>
> > @@ -48,6 +49,26 @@ protected:
> >  			return TestFail;
> >  		}
> >  
> > +		std::array<bool, 2> bools{ true, false };
> > +		value.set(Span<bool>(bools));
> > +		if (value.isNone() || !value.isArray() ||
> > +		    value.type() != ControlTypeBool) {
> > +			cerr << "Control type mismatch after setting to bool array" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		Span<const bool> boolsResult = value.get<Span<const bool>>();
> > +		if (bools.size() != boolsResult.size() ||
> > +		    !std::equal(bools.begin(), bools.end(), boolsResult.begin())) {
> > +			cerr << "Control value mismatch after setting to bool" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (value.toString() != "[ true, false ]") {
> > +			cerr << "Control string mismatch after setting to bool array" << endl;
> > +			return TestFail;
> > +		}
> > +
> >  		/*
> >  		 * Integer8 type.
> >  		 */
> > @@ -68,6 +89,26 @@ protected:
> >  			return TestFail;
> >  		}
> >  
> > +		std::array<uint8_t, 4> bytes{ 3, 14, 15, 9 };
> > +		value.set(Span<uint8_t>(bytes));
> > +		if (value.isNone() || !value.isArray() ||
> > +		    value.type() != ControlTypeByte) {
> > +			cerr << "Control type mismatch after setting to uint8_t array" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		Span<const uint8_t> int8sResult = value.get<Span<const uint8_t>>();
> > +		if (bytes.size() != int8sResult.size() ||
> > +		    !std::equal(bytes.begin(), bytes.end(), int8sResult.begin())) {
> > +			cerr << "Control value mismatch after setting to uint8_t array" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (value.toString() != "[ 3, 14, 15, 9 ]") {
> > +			cerr << "Control string mismatch after setting to uint8_t array" << endl;
> > +			return TestFail;
> > +		}
> > +
> >  		/*
> >  		 * Integer32 type.
> >  		 */
> > @@ -88,6 +129,26 @@ protected:
> >  			return TestFail;
> >  		}
> >  
> > +		std::array<int32_t, 4> int32s{ 3, 14, 15, 9 };
> > +		value.set(Span<int32_t>(int32s));
> > +		if (value.isNone() || !value.isArray() ||
> > +		    value.type() != ControlTypeInteger32) {
> > +			cerr << "Control type mismatch after setting to int32_t array" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		Span<const int32_t> int32sResult = value.get<Span<const int32_t>>();
> > +		if (int32s.size() != int32sResult.size() ||
> > +		    !std::equal(int32s.begin(), int32s.end(), int32sResult.begin())) {
> > +			cerr << "Control value mismatch after setting to int32_t array" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (value.toString() != "[ 3, 14, 15, 9 ]") {
> > +			cerr << "Control string mismatch after setting to int32_t array" << endl;
> > +			return TestFail;
> > +		}
> > +
> >  		/*
> >  		 * Integer64 type.
> >  		 */
> > @@ -108,6 +169,26 @@ protected:
> >  			return TestFail;
> >  		}
> >  
> > +		std::array<int64_t, 4> int64s{ 3, 14, 15, 9 };
> > +		value.set(Span<int64_t>(int64s));
> > +		if (value.isNone() || !value.isArray() ||
> > +		    value.type() != ControlTypeInteger64) {
> > +			cerr << "Control type mismatch after setting to int64_t array" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		Span<const int64_t> int64sResult = value.get<Span<const int64_t>>();
> > +		if (int64s.size() != int64sResult.size() ||
> > +		    !std::equal(int64s.begin(), int64s.end(), int64sResult.begin())) {
> > +			cerr << "Control value mismatch after setting to int64_t array" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (value.toString() != "[ 3, 14, 15, 9 ]") {
> > +			cerr << "Control string mismatch after setting to int64_t array" << endl;
> > +			return TestFail;
> > +		}
> > +
> >  		/*
> >  		 * Float type.
> >  		 */
> > @@ -128,6 +209,26 @@ protected:
> >  			return TestFail;
> >  		}
> >  
> > +		std::array<float, 3> floats{ 3.141593, 2.718282, 299792458.0 };
> 
> Storing three floats...
> 
> > +		value.set(Span<float>(floats));
> > +		if (value.isNone() || !value.isArray() ||
> > +		    value.type() != ControlTypeFloat) {
> > +			cerr << "Control type mismatch after setting to float array" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		Span<const float> floatsResult = value.get<Span<const float>>();
> > +		if (floats.size() != floatsResult.size() ||
> > +		    !std::equal(floats.begin(), floats.end(), floatsResult.begin())) {
> > +			cerr << "Control value mismatch after setting to float array" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (value.toString() != "[ 3.141593, 2.718282, 299792448.000000 ]") {
> 
> So we stored three floats including: 299792458.0 ...
> But validated that against         : 299792448.000000
> 
> I presume that this will be due to floating point precision, but the
> fact we store one value and check 'another' is a bit ugly, and either
> warrants a comment to make it explicit (and not 'hidden') or just use a
> value which will not lose any precision ?

I'll add a comment.

> But with that resolved how you wish, I think this is the last tag?
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> > +			cerr << "Control string mismatch after setting to float array" << endl;
> > +			return TestFail;
> > +		}
> > +
> >  		return TestPass;
> >  	}
> >  };

-- 
Regards,

Laurent Pinchart