On Fri, Mar 11, 2022 at 11:24 AM Vincent Whitchurch <vincent.whitchurch@xxxxxxxx> wrote: > > Add a regression test for the problem fixed by the following patch, > which would require specific environmental conditions to be able to be > reproduced and regression-tested on real hardware: > > iio: light: opt3001: Fixed timeout error when 0 lux > https://lore.kernel.org/lkml/20210920125351.6569-1-valek@xxxxx/ > > No other aspects of the driver are tested. > > Signed-off-by: Vincent Whitchurch <vincent.whitchurch@xxxxxxxx> > --- > .../roadtest/roadtest/tests/iio/__init__.py | 0 > .../roadtest/roadtest/tests/iio/config | 1 + > .../roadtest/tests/iio/light/__init__.py | 0 > .../roadtest/roadtest/tests/iio/light/config | 1 + > .../roadtest/tests/iio/light/test_opt3001.py | 95 +++++++++++++++++++ > 5 files changed, 97 insertions(+) > create mode 100644 tools/testing/roadtest/roadtest/tests/iio/__init__.py > create mode 100644 tools/testing/roadtest/roadtest/tests/iio/config > create mode 100644 tools/testing/roadtest/roadtest/tests/iio/light/__init__.py > create mode 100644 tools/testing/roadtest/roadtest/tests/iio/light/config > create mode 100644 tools/testing/roadtest/roadtest/tests/iio/light/test_opt3001.py > > diff --git a/tools/testing/roadtest/roadtest/tests/iio/__init__.py b/tools/testing/roadtest/roadtest/tests/iio/__init__.py > new file mode 100644 > index 000000000000..e69de29bb2d1 > diff --git a/tools/testing/roadtest/roadtest/tests/iio/config b/tools/testing/roadtest/roadtest/tests/iio/config > new file mode 100644 > index 000000000000..a08d9e23ce38 > --- /dev/null > +++ b/tools/testing/roadtest/roadtest/tests/iio/config > @@ -0,0 +1 @@ > +CONFIG_IIO=y > diff --git a/tools/testing/roadtest/roadtest/tests/iio/light/__init__.py b/tools/testing/roadtest/roadtest/tests/iio/light/__init__.py > new file mode 100644 > index 000000000000..e69de29bb2d1 > diff --git a/tools/testing/roadtest/roadtest/tests/iio/light/config b/tools/testing/roadtest/roadtest/tests/iio/light/config > new file mode 100644 > index 000000000000..b9753f2d0728 > --- /dev/null > +++ b/tools/testing/roadtest/roadtest/tests/iio/light/config > @@ -0,0 +1 @@ > +CONFIG_OPT3001=m > diff --git a/tools/testing/roadtest/roadtest/tests/iio/light/test_opt3001.py b/tools/testing/roadtest/roadtest/tests/iio/light/test_opt3001.py > new file mode 100644 > index 000000000000..abf20b8f3516 > --- /dev/null > +++ b/tools/testing/roadtest/roadtest/tests/iio/light/test_opt3001.py > @@ -0,0 +1,95 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +# Copyright Axis Communications AB > + > +from typing import Any, Final > + > +from roadtest.backend.i2c import SMBusModel > +from roadtest.core.devicetree import DtFragment, DtVar > +from roadtest.core.hardware import Hardware > +from roadtest.core.modules import insmod, rmmod > +from roadtest.core.suite import UMLTestCase > +from roadtest.core.sysfs import I2CDriver, read_float > + > +REG_RESULT: Final = 0x00 > +REG_CONFIGURATION: Final = 0x01 > +REG_LOW_LIMIT: Final = 0x02 > +REG_HIGH_LIMIT: Final = 0x03 > +REG_MANUFACTURER_ID: Final = 0x7E > +REG_DEVICE_ID: Final = 0x7F > + > +REG_CONFIGURATION_CRF: Final = 1 << 7 > + > + > +class OPT3001(SMBusModel): > + def __init__(self, **kwargs: Any) -> None: > + super().__init__(regbytes=2, byteorder="big", **kwargs) > + # Reset values from datasheet > + self.regs = { > + REG_RESULT: 0x0000, > + REG_CONFIGURATION: 0xC810, > + REG_LOW_LIMIT: 0xC000, > + REG_HIGH_LIMIT: 0xBFFF, > + REG_MANUFACTURER_ID: 0x5449, > + REG_DEVICE_ID: 0x3001, > + } > + > + def reg_read(self, addr: int) -> int: > + val = self.regs[addr] > + > + if addr == REG_CONFIGURATION: > + # Always indicate that the conversion is ready. This is good > + # enough for our current purposes. > + val |= REG_CONFIGURATION_CRF > + > + return val > + > + def reg_write(self, addr: int, val: int) -> None: > + assert addr in self.regs > + self.regs[addr] = val > + > + > +class TestOPT3001(UMLTestCase): I am partial to starting with UML since there are a lot of nice easy things about starting there; however, I imagine people will eventually want to use this on other architectures (speaking from experience). How difficult do you think it would be to extend this to support manipulating fake devices in say QEMU? I also have some colleagues inside of Google that worked on some projects to simulate simple devices on an FPGA to test software and adjacent devices in a conceptually similar way; one of these teams built a Domain Specific Language kind of like roadtest to implement the tests and the environment for the tests. The main reason I mention this here is I am thinking about maybe one day having an API you can implement so you can run your roadtests on UML, QEMU, or on any emulator or hardware testbed that implements the appropriate API. I'll try to dig up some people who might be interested and add them here. > + dts = DtFragment( > + src=""" > +&i2c { > + light-sensor@$addr$ { > + compatible = "ti,opt3001"; > + reg = <0x$addr$>; > + }; > +}; > + """, > + variables={ > + "addr": DtVar.I2C_ADDR, > + }, > + ) > + > + @classmethod > + def setUpClass(cls) -> None: > + insmod("opt3001") > + > + @classmethod > + def tearDownClass(cls) -> None: > + rmmod("opt3001") > + > + def setUp(self) -> None: > + self.driver = I2CDriver("opt3001") > + self.hw = Hardware("i2c") > + self.hw.load_model(OPT3001) > + > + def tearDown(self) -> None: > + self.hw.close() > + > + def test_illuminance(self) -> None: > + data = [ > + # Some values from datasheet, and 0 > + (0b_0000_0000_0000_0000, 0), > + (0b_0000_0000_0000_0001, 0.01), > + (0b_0011_0100_0101_0110, 88.80), > + (0b_0111_1000_1001_1010, 2818.56), > + ] > + with self.driver.bind(self.dts["addr"]) as dev: > + luxfile = dev.path / "iio:device0/in_illuminance_input" > + > + for regval, lux in data: > + self.hw.reg_write(REG_RESULT, regval) > + self.assertEqual(read_float(luxfile), lux) I love the framework; this looks very easy to use. One nit about this test; it seems like you cover just one test case here - the happy path. Can you cover some other one? Particularly some error paths? Sorry, I am not trying to be cheeky here; it looks like this driver actually should probably be fully (or very close to fully) testable via roadtest as I understand it. It only looks like there are a handful of cases to cover for the driver: the device is busy, the device returned something invalid, the user requested something invalid, and several SMBus read/write failures - it really only looks like there are a handful of paths and I think they are all accessible via the I2C interface (except for maybe the user requesting something invalid).
