[PATCH v2 0/3] cmd/button: return button status

[PATCH v2 0/3] cmd/button: return button status

[Heinrich Schuchardt]

To make the button command useful in a shell script it should return the
status of the button.

Adjust the sandbox GPIO driver to keep the output value as input when
switching to input.

Adjust the device-tree labels of the sandbox GPIOs used for buttons.

Enhance the Python unit test to check the button status returned by the
button command.

v2:
	adjust Python test
	adjust GPIO driver

Heinrich Schuchardt (3):
  test: sharpen button label unit test
  drivers: gpio: keep output value for input on sandbox
  cmd/button: return button status

 arch/sandbox/dts/sandbox.dtsi |  8 ++++----
 arch/sandbox/dts/test.dts     |  8 ++++----
 cmd/button.c                  |  4 ++--
 drivers/gpio/sandbox.c        | 10 +++++++++-
 test/dm/button.c              |  6 +++---
 test/py/tests/test_button.py  | 36 ++++++++++++++++++++++++++---------
 6 files changed, 49 insertions(+), 23 deletions(-)

--
2.28.0

[PATCH v2 1/3] test: sharpen button label unit test

[Heinrich Schuchardt]

Using different strings for the device tree node labels and the label
property of buttons sharpens the button label unit test.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2:
	new patch
---
 arch/sandbox/dts/sandbox.dtsi | 8 ++++----
 arch/sandbox/dts/test.dts     | 8 ++++----
 test/dm/button.c              | 6 +++---
 test/py/tests/test_button.py  | 2 +-
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi
index c76ecc013c..0faad3f319 100644
--- a/arch/sandbox/dts/sandbox.dtsi
+++ b/arch/sandbox/dts/sandbox.dtsi
@@ -18,14 +18,14 @@
 	buttons {
 		compatible = "gpio-keys";

-		summer {
+		btn1 {
 			gpios = <&gpio_a 3 0>;
-			label = "summer";
+			label = "button1";
 		};

-		christmas {
+		btn2 {
 			gpios = <&gpio_a 4 0>;
-			label = "christmas";
+			label = "button2";
 		};
 	};

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 9f45c48e4e..395d5f5e56 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -54,14 +54,14 @@
 	buttons {
 		compatible = "gpio-keys";

-		summer {
+		btn1 {
 			gpios = <&gpio_a 3 0>;
-			label = "summer";
+			label = "button1";
 		};

-		christmas {
+		btn2 {
 			gpios = <&gpio_a 4 0>;
-			label = "christmas";
+			label = "button2";
 		};
 	};

diff --git a/test/dm/button.c b/test/dm/button.c
index 9117801736..ecaa47cf5f 100644
--- a/test/dm/button.c
+++ b/test/dm/button.c
@@ -57,17 +57,17 @@ static int dm_test_button_label(struct unit_test_state *uts)
 {
 	struct udevice *dev, *cmp;

-	ut_assertok(button_get_by_label("summer", &dev));
+	ut_assertok(button_get_by_label("button1", &dev));
 	ut_asserteq(1, device_active(dev));
 	ut_assertok(uclass_get_device(UCLASS_BUTTON, 1, &cmp));
 	ut_asserteq_ptr(dev, cmp);

-	ut_assertok(button_get_by_label("christmas", &dev));
+	ut_assertok(button_get_by_label("button2", &dev));
 	ut_asserteq(1, device_active(dev));
 	ut_assertok(uclass_get_device(UCLASS_BUTTON, 2, &cmp));
 	ut_asserteq_ptr(dev, cmp);

-	ut_asserteq(-ENODEV, button_get_by_label("spring", &dev));
+	ut_asserteq(-ENODEV, button_get_by_label("nobutton", &dev));

 	return 0;
 }
diff --git a/test/py/tests/test_button.py b/test/py/tests/test_button.py
index 98067a98f2..eadd9dd613 100644
--- a/test/py/tests/test_button.py
+++ b/test/py/tests/test_button.py
@@ -11,7 +11,7 @@ def test_button_exit_statuses(u_boot_console):
     expected_response = 'rc:0'
     response = u_boot_console.run_command('button list; echo rc:$?')
     assert(expected_response in response)
-    response = u_boot_console.run_command('button summer; echo rc:$?')
+    response = u_boot_console.run_command('button button1; echo rc:$?')
     assert(expected_response in response)

     expected_response = 'rc:1'
--
2.28.0

[PATCH v2 1/3] test: sharpen button label unit test

[Philippe REYNES]

> Using different strings for the device tree node labels and the label
> property of buttons sharpens the button label unit test.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Reviewed-by: Philippe Reynes <philippe.reynes@softathome.com>

> ---
> v2:
> new patch
> ---
> arch/sandbox/dts/sandbox.dtsi | 8 ++++----
> arch/sandbox/dts/test.dts | 8 ++++----
> test/dm/button.c | 6 +++---
> test/py/tests/test_button.py | 2 +-
> 4 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi
> index c76ecc013c..0faad3f319 100644
> --- a/arch/sandbox/dts/sandbox.dtsi
> +++ b/arch/sandbox/dts/sandbox.dtsi
> @@ -18,14 +18,14 @@
> buttons {
> compatible = "gpio-keys";
> 
> - summer {
> + btn1 {
> gpios = <&gpio_a 3 0>;
> - label = "summer";
> + label = "button1";
> };
> 
> - christmas {
> + btn2 {
> gpios = <&gpio_a 4 0>;
> - label = "christmas";
> + label = "button2";
> };
> };
> 
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> index 9f45c48e4e..395d5f5e56 100644
> --- a/arch/sandbox/dts/test.dts
> +++ b/arch/sandbox/dts/test.dts
> @@ -54,14 +54,14 @@
> buttons {
> compatible = "gpio-keys";
> 
> - summer {
> + btn1 {
> gpios = <&gpio_a 3 0>;
> - label = "summer";
> + label = "button1";
> };
> 
> - christmas {
> + btn2 {
> gpios = <&gpio_a 4 0>;
> - label = "christmas";
> + label = "button2";
> };
> };
> 
> diff --git a/test/dm/button.c b/test/dm/button.c
> index 9117801736..ecaa47cf5f 100644
> --- a/test/dm/button.c
> +++ b/test/dm/button.c
> @@ -57,17 +57,17 @@ static int dm_test_button_label(struct unit_test_state *uts)
> {
> struct udevice *dev, *cmp;
> 
> - ut_assertok(button_get_by_label("summer", &dev));
> + ut_assertok(button_get_by_label("button1", &dev));
> ut_asserteq(1, device_active(dev));
> ut_assertok(uclass_get_device(UCLASS_BUTTON, 1, &cmp));
> ut_asserteq_ptr(dev, cmp);
> 
> - ut_assertok(button_get_by_label("christmas", &dev));
> + ut_assertok(button_get_by_label("button2", &dev));
> ut_asserteq(1, device_active(dev));
> ut_assertok(uclass_get_device(UCLASS_BUTTON, 2, &cmp));
> ut_asserteq_ptr(dev, cmp);
> 
> - ut_asserteq(-ENODEV, button_get_by_label("spring", &dev));
> + ut_asserteq(-ENODEV, button_get_by_label("nobutton", &dev));
> 
> return 0;
> }
> diff --git a/test/py/tests/test_button.py b/test/py/tests/test_button.py
> index 98067a98f2..eadd9dd613 100644
> --- a/test/py/tests/test_button.py
> +++ b/test/py/tests/test_button.py
> @@ -11,7 +11,7 @@ def test_button_exit_statuses(u_boot_console):
> expected_response = 'rc:0'
> response = u_boot_console.run_command('button list; echo rc:$?')
> assert(expected_response in response)
> - response = u_boot_console.run_command('button summer; echo rc:$?')
> + response = u_boot_console.run_command('button button1; echo rc:$?')
> assert(expected_response in response)
> 
> expected_response = 'rc:1'
> --
> 2.28.0

[PATCH v2 1/3] test: sharpen button label unit test

[Tom Rini]

On Mon, Sep 14, 2020 at 12:50:54PM +0200, Heinrich Schuchardt wrote:

> Using different strings for the device tree node labels and the label
> property of buttons sharpens the button label unit test.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Reviewed-by: Philippe Reynes <philippe.reynes@softathome.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201014/08775513/attachment.sig>

[PATCH v2 2/3] drivers: gpio: keep output value for input on sandbox

[Heinrich Schuchardt]

For testing purposes keep the output value when switching to input.
This allows us to manipulate the input value via the gpio command.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2:
	new patch
---
 drivers/gpio/sandbox.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c
index c2f80472b8..eb2600de31 100644
--- a/drivers/gpio/sandbox.c
+++ b/drivers/gpio/sandbox.c
@@ -185,7 +185,15 @@ static int sb_gpio_set_dir_flags(struct udevice *dev, unsigned int offset,

 	dir_flags = get_gpio_dir_flags(dev, offset);

-	*dir_flags = flags;
+	/*
+	 * For testing purposes keep the output value when switching to input.
+	 * This allows us to manipulate the input value via the gpio command.
+	 */
+	if (flags & GPIOD_IS_IN)
+		*dir_flags = (flags & ~GPIOD_IS_OUT_ACTIVE) |
+			     (*dir_flags & GPIOD_IS_OUT_ACTIVE);
+	else
+		*dir_flags = flags;

 	return 0;
 }
--
2.28.0

[PATCH v2 2/3] drivers: gpio: keep output value for input on sandbox

[Philippe REYNES]

> For testing purposes keep the output value when switching to input.
> This allows us to manipulate the input value via the gpio command.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Reviewed-by: Philippe Reynes <philippe.reynes@softathome.com>

> ---
> v2:
> new patch
> ---
> drivers/gpio/sandbox.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c
> index c2f80472b8..eb2600de31 100644
> --- a/drivers/gpio/sandbox.c
> +++ b/drivers/gpio/sandbox.c
> @@ -185,7 +185,15 @@ static int sb_gpio_set_dir_flags(struct udevice *dev,
> unsigned int offset,
> 
> dir_flags = get_gpio_dir_flags(dev, offset);
> 
> - *dir_flags = flags;
> + /*
> + * For testing purposes keep the output value when switching to input.
> + * This allows us to manipulate the input value via the gpio command.
> + */
> + if (flags & GPIOD_IS_IN)
> + *dir_flags = (flags & ~GPIOD_IS_OUT_ACTIVE) |
> + (*dir_flags & GPIOD_IS_OUT_ACTIVE);
> + else
> + *dir_flags = flags;
> 
> return 0;
> }
> --
> 2.28.0

[PATCH v2 2/3] drivers: gpio: keep output value for input on sandbox

[Tom Rini]

On Mon, Sep 14, 2020 at 12:50:55PM +0200, Heinrich Schuchardt wrote:

> For testing purposes keep the output value when switching to input.
> This allows us to manipulate the input value via the gpio command.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Reviewed-by: Philippe Reynes <philippe.reynes@softathome.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201014/b8f1de05/attachment.sig>

[PATCH v2 3/3] cmd/button: return button status

[Heinrich Schuchardt]

To make the button command useful in a shell script it should return the
status of the button:

* 0 (true) - pressed, on
* 1 (false) - not pressed, off

The button command takes only one argument. Correct maxargs.

Adjust the Python unit test.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2:
	adjust Python unit test
---
 cmd/button.c                 |  4 ++--
 test/py/tests/test_button.py | 34 ++++++++++++++++++++++++++--------
 2 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/cmd/button.c b/cmd/button.c
index 84ad1653c7..64c5a8fa04 100644
--- a/cmd/button.c
+++ b/cmd/button.c
@@ -75,11 +75,11 @@ int do_button(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])

 	ret = show_button_state(dev);

-	return 0;
+	return !ret;
 }

 U_BOOT_CMD(
-	button, 4, 1, do_button,
+	button, 2, 1, do_button,
 	"manage buttons",
 	"<button_label> \tGet button state\n"
 	"button list\t\tShow a list of buttons"
diff --git a/test/py/tests/test_button.py b/test/py/tests/test_button.py
index eadd9dd613..3b7f148c8f 100644
--- a/test/py/tests/test_button.py
+++ b/test/py/tests/test_button.py
@@ -4,16 +4,34 @@ import pytest

 @pytest.mark.boardspec('sandbox')
 @pytest.mark.buildconfigspec('cmd_button')
-def test_button_exit_statuses(u_boot_console):
-    """Test that non-input button commands correctly return the command
-    success/failure status."""
+def test_button_list(u_boot_console):
+    """Test listing buttons"""

-    expected_response = 'rc:0'
     response = u_boot_console.run_command('button list; echo rc:$?')
-    assert(expected_response in response)
+    assert('button1' in response)
+    assert('button2' in response)
+    assert('rc:0' in response)
+
+ at pytest.mark.boardspec('sandbox')
+ at pytest.mark.buildconfigspec('cmd_button')
+ at pytest.mark.buildconfigspec('cmd_gpio')
+def test_button_return_code(u_boot_console):
+    """Test correct reporting of the button status
+
+    The sandbox gpio driver reports the last output value as input value.
+    We can use this in our test to emulate different input statuses.
+    """
+
+    u_boot_console.run_command('gpio set a3; gpio input a3');
+    response = u_boot_console.run_command('button button1; echo rc:$?')
+    assert('on' in response)
+    assert('rc:0' in response)
+
+    u_boot_console.run_command('gpio clear a3; gpio input a3');
     response = u_boot_console.run_command('button button1; echo rc:$?')
-    assert(expected_response in response)
+    assert('off' in response)
+    assert('rc:1' in response)

-    expected_response = 'rc:1'
     response = u_boot_console.run_command('button nonexistent-button; echo rc:$?')
-    assert(expected_response in response)
+    assert('not found' in response)
+    assert('rc:1' in response)
--
2.28.0

[PATCH v2 3/3] cmd/button: return button status

[Philippe REYNES]

> To make the button command useful in a shell script it should return the
> status of the button:
> 
> * 0 (true) - pressed, on
> * 1 (false) - not pressed, off
> 
> The button command takes only one argument. Correct maxargs.
> 
> Adjust the Python unit test.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Reviewed-by: Philippe Reynes <philippe.reynes@softathome.com>

> ---
> v2:
> adjust Python unit test
> ---
> cmd/button.c | 4 ++--
> test/py/tests/test_button.py | 34 ++++++++++++++++++++++++++--------
> 2 files changed, 28 insertions(+), 10 deletions(-)
> 
> diff --git a/cmd/button.c b/cmd/button.c
> index 84ad1653c7..64c5a8fa04 100644
> --- a/cmd/button.c
> +++ b/cmd/button.c
> @@ -75,11 +75,11 @@ int do_button(struct cmd_tbl *cmdtp, int flag, int argc,
> char *const argv[])
> 
> ret = show_button_state(dev);
> 
> - return 0;
> + return !ret;
> }
> 
> U_BOOT_CMD(
> - button, 4, 1, do_button,
> + button, 2, 1, do_button,
> "manage buttons",
> "<button_label> \tGet button state\n"
> "button list\t\tShow a list of buttons"
> diff --git a/test/py/tests/test_button.py b/test/py/tests/test_button.py
> index eadd9dd613..3b7f148c8f 100644
> --- a/test/py/tests/test_button.py
> +++ b/test/py/tests/test_button.py
> @@ -4,16 +4,34 @@ import pytest
> 
> @pytest.mark.boardspec('sandbox')
> @pytest.mark.buildconfigspec('cmd_button')
> -def test_button_exit_statuses(u_boot_console):
> - """Test that non-input button commands correctly return the command
> - success/failure status."""
> +def test_button_list(u_boot_console):
> + """Test listing buttons"""
> 
> - expected_response = 'rc:0'
> response = u_boot_console.run_command('button list; echo rc:$?')
> - assert(expected_response in response)
> + assert('button1' in response)
> + assert('button2' in response)
> + assert('rc:0' in response)
> +
> + at pytest.mark.boardspec('sandbox')
> + at pytest.mark.buildconfigspec('cmd_button')
> + at pytest.mark.buildconfigspec('cmd_gpio')
> +def test_button_return_code(u_boot_console):
> + """Test correct reporting of the button status
> +
> + The sandbox gpio driver reports the last output value as input value.
> + We can use this in our test to emulate different input statuses.
> + """
> +
> + u_boot_console.run_command('gpio set a3; gpio input a3');
> + response = u_boot_console.run_command('button button1; echo rc:$?')
> + assert('on' in response)
> + assert('rc:0' in response)
> +
> + u_boot_console.run_command('gpio clear a3; gpio input a3');
> response = u_boot_console.run_command('button button1; echo rc:$?')
> - assert(expected_response in response)
> + assert('off' in response)
> + assert('rc:1' in response)
> 
> - expected_response = 'rc:1'
> response = u_boot_console.run_command('button nonexistent-button; echo rc:$?')
> - assert(expected_response in response)
> + assert('not found' in response)
> + assert('rc:1' in response)
> --
> 2.28.0

[PATCH v2 3/3] cmd/button: return button status

[Tom Rini]

On Mon, Sep 14, 2020 at 12:50:56PM +0200, Heinrich Schuchardt wrote:

> To make the button command useful in a shell script it should return the
> status of the button:
> 
> * 0 (true) - pressed, on
> * 1 (false) - not pressed, off
> 
> The button command takes only one argument. Correct maxargs.
> 
> Adjust the Python unit test.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Reviewed-by: Philippe Reynes <philippe.reynes@softathome.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201014/3144af59/attachment.sig>