[riot-notifications] [RIOT-OS/RIOT] WIP: tests/ieee802154_hal: extend radio hal test (#15761)

Martine Lenders notifications at github.com
Mon Jun 7 14:19:42 CEST 2021


@miri64 commented on this pull request.



> @@ -0,0 +1,98 @@
+# Copyright (C) 2019-20 Freie Universität Berlin

Please move this whole file content to your application's test script, as it only implements shell interactions for application-specfic shell-commands.

> @@ -0,0 +1,98 @@
+# Copyright (C) 2019-20 Freie Universität Berlin
+#
+# This file is subject to the terms and conditions of the GNU Lesser
+# General Public License v2.1. See the file LICENSE in the top level
+# directory for more details.
+
+import re
+
+from riotctrl.shell import ShellInteraction
+
+# ==== ShellInteractions ====
+
+class Config_phy(ShellInteraction):

All these shell interactions can be one class IMHO. e.g. `IEEE802154Phy(ShellInteraction):`

> @@ -0,0 +1,98 @@
+# Copyright (C) 2019-20 Freie Universität Berlin
+#
+# This file is subject to the terms and conditions of the GNU Lesser
+# General Public License v2.1. See the file LICENSE in the top level
+# directory for more details.
+
+import re
+
+from riotctrl.shell import ShellInteraction
+
+# ==== ShellInteractions ====
+
+class Config_phy(ShellInteraction):
+    def config_phy(self, phy_mode, channel, tx_pow, timeout=None, async_=False):

This should be prefixed properly, as other shell interactions might configure other PHYs (e.g. LoRaWAN):

```suggestion
    def ieee802154_config_phy(self, phy_mode, channel, tx_pow, timeout=None, async_=False):
```

> +              .format(phy_mode=phy_mode, channel=channel, tx_pow=tx_pow)
+        
+        try:
+            res = self.cmd(cmd, timeout=timeout, async_=async_)
+        except Exception as e:
+            print(str(e))
+            print("Exception")
+        
+        if "Success" not in res:
+            raise RuntimeError(res)
+
+        if str(channel) not in res:
+            raise RuntimeError(res)
+
+class Print_addr(ShellInteraction):
+    def print_addr(self, timeout=None, async_=False):

Same here
```suggestion
    def ieee802154_print_addr(self, timeout=None, async_=False):
```

> +            res = self.cmd(cmd, timeout=timeout, async_=async_)
+        except Exception as e:
+            print(str(e))
+            print("Exception")
+
+        count = 0
+        for line in res.splitlines():
+            count+=1
+            if count > 2:
+                break
+            pass
+        addr = line
+        return addr
+
+class Txtsnd(ShellInteraction):
+    def tx_mode(self, command, timeout=None, async_=False):

And here and below.

> +            print(str(e))
+            print("Exception")

Redundant output and cast usage. How about just:

```suggestion
            print("Error:", e)
```

Also, never just catch `Exception` but specific ones. [`pylint`](https://pylint.pycqa.org/en/latest/) or [`flake8`](https://flake8.pycqa.org/en/latest/) will tell you as much.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/15761#pullrequestreview-677334040
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210607/b132b7be/attachment.htm>


More information about the notifications mailing list