[riot-notifications] [RIOT-OS/RIOT] cpu/stm32_common: add watchdog for stm32 (#11252)

Alexandre Abadie notifications at github.com
Tue May 14 07:16:28 CEST 2019


aabadie requested changes on this pull request.

The test application and script still require some improvements. See below.

> @@ -0,0 +1,61 @@
+#!/usr/bin/env python3
+
+# Copyright (C) 2019 Inria
+#
+# 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 sys
+import json
+from testrunner import run
+
+# We test only up to 10ms, with smaller times mcu doesn't have time to
+# print sysyem time before resetting

typo: should be sys**t**em instead of sys**y**em

> +
+def get_start_time(child):
+    child.readline()
+    child.readline()
+    line = child.readline().strip().split("json:")
+    start_time = json.loads(line[1])
+    return start_time['starttime']
+
+
+def testfunc(child):
+    for rst_time in reset_times_us:
+        child.sendline("setup 0 {}".format(rst_time))
+        child.expect_exact("[wdg]: - ")
+        child.expect_exact(" -")
+
+        if child.before != "invalid configuration time":

You don't need this check. This (minor) case can simply be tested sending and invalid configuration using `sendline` and test the output is as expect using `expect_exact` or a regexp in `expect`.

> +    child.readline()
+    line = child.readline().strip().split("json:")
+    start_time = json.loads(line[1])
+    return start_time['starttime']
+
+
+def testfunc(child):
+    for rst_time in reset_times_us:
+        child.sendline("setup 0 {}".format(rst_time))
+        child.expect_exact("[wdg]: - ")
+        child.expect_exact(" -")
+
+        if child.before != "invalid configuration time":
+            child.sendline("startloop")
+            start_time = get_start_time(child)
+            reset_time = get_reset_time(child)

I find `get_reset_time` overly complex. This could be much much simpler using `child.expect` and regex to get the reset time. I also don't like the use of json in the output of the application.
Have a look at how numerical values are retrieved using regexp in [the python script periph_rtt](https://github.com/RIOT-OS/RIOT/blob/master/tests/periph_rtt/tests/01-run.py#L20). This can be done the same way (not tested):
```python
child.expect(r'  Reset time: (\d+)')
reset_time = int(child.match.group(1))
```

Same for start time:
```python
child.expect(r'  Start time: (\d+)')
start_time = int(child.match.group(1))
```

-- 
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/11252#pullrequestreview-237027557
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190513/738f4a46/attachment.html>


More information about the notifications mailing list