Merge #19546 #20022 #20025

19546: Enable compile_and_test_for_board to skip if nothing changed r=benpicco a=MrKevinWeiss ### Contribution description The overall goal of this PR is to be able to keep running the `compile_and_test_for_board.py` script without destroying ones boards. Useful if you are a board owner that wants to keep testing on master (say with nightlies, say in a CI). For example, I could now just run and rerun the following: ``` ./dist/tools/compile_and_test_for_board/compile_and_test_for_board.py . <MY_BOARD> -c ``` and monitor the results, even after updating a branch. This is because the hashes now get stored with the results (thanks to the `RIOT_TEST_HASH_DIR` var) which means cleaning will not wipe them out. Specifically in (`<results_base>/<board>/hashes/<app_dir>/test-input-hash.sha1`) I tried to do as much as I could with make and only alter the python script when needed. ### Testing procedure Clear results folder and run: ``` ./dist/tools/compile_and_test_for_board/compile_and_test_for_board.py . native --applications tests/shell -c ``` <details> ``` INFO:native:Saving toolchain INFO:native.tests/shell:Board supported: True INFO:native.tests/shell:Board has enough memory: True INFO:native.tests/shell:Application has test: True INFO:native.tests/shell:Run compilation **INFO:native.tests/shell:Hashes match: False** INFO:native.tests/shell:Run test INFO:native.tests/shell:Run test.flash INFO:native.tests/shell:Success INFO:native:Tests successful ``` </details> it should execute the test... Then the same command again should skip it. <details> ``` INFO:native:Saving toolchain INFO:native.tests/shell:Board supported: True INFO:native.tests/shell:Board has enough memory: True INFO:native.tests/shell:Application has test: True INFO:native.tests/shell:Run compilation **INFO:native.tests/shell:Hashes match: True** INFO:native.tests/shell:Success INFO:native:Tests successful ``` </details> Try changing something (say the `tests/shell/01-run.py`) and rerun, it should trigger the test again. <details> ``` INFO:native:Saving toolchain INFO:native.tests/shell:Board supported: True INFO:native.tests/shell:Board has enough memory: True INFO:native.tests/shell:Application has test: True INFO:native.tests/shell:Run compilation **INFO:native.tests/shell:Hashes match: False** INFO:native.tests/shell:Run test INFO:native.tests/shell:Run test.flash INFO:native.tests/shell:Success INFO:native:Tests successful ``` </details> ...finally, running without the changes check should run the test as usual, even if nothing changes: ``` ./dist/tools/compile_and_test_for_board/compile_and_test_for_board.py . native --applications tests/shell ``` <details> ``` INFO:native:Saving toolchain INFO:native.tests/shell:Board supported: True INFO:native.tests/shell:Board has enough memory: True INFO:native.tests/shell:Application has test: True INFO:native.tests/shell:Run compilation INFO:native.tests/shell:Run test INFO:native.tests/shell:Run test.flash INFO:native.tests/shell:Success INFO:native:Tests successful ``` </details> ### Issues/PRs references Might help the surprises we got with #19469 20022: pkg/lwip: add support for slipdev r=benpicco a=benpicco 20025: tests/drivers/at: fix device table overflow r=benpicco a=krzysztof-cabaj ### Contribution description This PR fix device table overflow in `tests/driver/at`, which could lead to device crash. ### Testing procedure PR was tested on two nucleo boards with 2 and 3 UARTs (nucleo-l476rg and nucleo-l496zg). Flash `tests/driver/at` with and without this PR. Output with this PR: ``` > main(): This is RIOT! (Version: 2022.07-devel-5083-g2b9e8-tests-drivers-at) AT command test app > init 5 9600 Wrong UART device number - should by in range 0-2. > ``` Output without this PR: ``` > main(): This is RIOT! (Version: 2022.07-devel-5083-g2b9e8) AT command test app > init 5 9600 8001afd *** RIOT kernel panic: FAILED ASSERTION. *** halted. Context before hardfault: r0: 0x0000000a r1: 0x00000000 . . . ``` ### Issues/PRs references None Co-authored-by: MrKevinWeiss <[email protected]> Co-authored-by: Benjamin Valentin <[email protected]> Co-authored-by: krzysztof-cabaj <[email protected]>
RIOT-OS · Oct 27, 2023 · 86bab88 · 86bab88
4 parents 47f0446 + a439442 + 32d17f5 + a32dbbf
commit 86bab88
Show file tree

Hide file tree

Showing 7 changed files with 205 additions and 5 deletions.
diff --git a/dist/tools/compile_and_test_for_board/compile_and_test_for_board.py b/dist/tools/compile_and_test_for_board/compile_and_test_for_board.py
@@ -44,6 +44,7 @@
                                      [--test-targets TEST_TARGETS]
                                      [--test-available-targets TEST_AVAILABLE_TARGETS]
                                      [--report-xml] [--jobs JOBS]
+                                     [--only-if-changed]
                                      riot_directory board [result_directory]
 
 positional arguments:
@@ -84,6 +85,9 @@
                         (default: False)
   --jobs JOBS, -j JOBS  Parallel building (0 means not limit, like '--jobs')
                         (default: None)
+  --only-if-changed, -c
+                        Only test if the application has changed since last
+                        test (default: False)
 ```
 """  # noqa
 
@@ -225,6 +229,7 @@ def is_in_directory(path, directory):
     return path.startswith(directory)
 
 
+# pylint: disable=too-many-instance-attributes
 class RIOTApplication:
     """RIOT Application representation.
 
@@ -245,13 +250,15 @@ class RIOTApplication:
     )
     FLASH_TARGETS = ("flash-only",)
     TEST_TARGETS = ("test",)
+    TEST_INPUT_HASH = ("test-input-hash-changed",)
     TEST_AVAILABLE_TARGETS = ("test/available",)
 
     # pylint: disable=too-many-arguments
     def __init__(self, board, riotdir, appdir, resultdir, junit=False):
         self.board = board
         self.riotdir = riotdir
         self.appdir = appdir
+        self.hashdir = os.path.abspath(os.path.join(resultdir, "hashes", appdir))
         self.resultdir = os.path.join(resultdir, appdir)
         if junit:
             if not junit_xml:
@@ -368,8 +375,11 @@ def compilation_and_test(
         incremental=False,
         jobs=False,
         with_test_only=False,
+        only_if_changed=False,
     ):
         # pylint:disable=too-many-arguments
+        # pylint:disable=too-many-nested-blocks
+        # pylint:disable=too-many-branches
         """Compile and execute test if available.
 
         Checks for board supported/enough memory, compiles.
@@ -422,8 +432,28 @@ def compilation_and_test(
         if clean_after:
             self.clean_intermediates()
 
+        # pylint: disable=too-many-nested-blocks
         if runtest:
             if has_test:
+                if only_if_changed:
+                    try:
+                        out = self.make(
+                            self.TEST_INPUT_HASH,
+                            env={"RIOT_TEST_HASH_DIR": self.hashdir},
+                        )
+                        hash_match = "hashes match" in out.lower()
+                        self.logger.info("Hashes match: %r", hash_match)
+                        if hash_match:
+                            self._skip(
+                                "bins_unchanged",
+                                f"{self.appdir} matched test input hashes.",
+                            )
+                            if clean_after:
+                                self.clean()
+                            self.logger.info("Success")
+                            return
+                    except subprocess.CalledProcessError as err:
+                        self._make_handle_error(self.TEST_INPUT_HASH[0], err)
                 setuptasks = collections.OrderedDict([("flash", self.FLASH_TARGETS)])
                 self.make_with_outfile(
                     "test", self.TEST_TARGETS, save_output=True, setuptasks=setuptasks
@@ -732,14 +762,21 @@ def _strip_board_equal(board):
     default=False,
     help="Output results to report.xml in the " "result_directory",
 )
-
 PARSER.add_argument(
     "--jobs",
     "-j",
     type=int,
     default=None,
     help="Parallel building (0 means not limit, like '--jobs')",
 )
+# Arg that allows hashes of tests to be saved locally in a hashes folder
+# and used to skip tests if new hashes match.
+PARSER.add_argument(
+    "--only-if-changed",
+    "-c",
+    action="store_true",
+    help="Only test if the application has changed since last test",
+)
 
 
 def main(args):
@@ -798,6 +835,7 @@ def main(args):
             incremental=args.incremental,
             jobs=args.jobs,
             with_test_only=args.with_test_only,
+            only_if_changed=args.only_if_changed,
         )
         for app in applications
     ]

diff --git a/drivers/slipdev/include/slipdev_params.h b/drivers/slipdev/include/slipdev_params.h
@@ -34,9 +34,13 @@ extern "C" {
  */
 #ifndef SLIPDEV_PARAM_UART
 # ifndef MODULE_SLIPDEV_STDIO
-#  define SLIPDEV_PARAM_UART        (UART_DEV(0))
+#  ifdef MODULE_USBUS_CDC_ACM
+#   define SLIPDEV_PARAM_UART       UART_DEV(0)
+#  else
+#   define SLIPDEV_PARAM_UART       UART_DEV(1)
+#  endif
 # else  /* MODULE_SLIPDEV_STDIO */
-#  define SLIPDEV_PARAM_UART        (STDIO_UART_DEV)
+#  define SLIPDEV_PARAM_UART        STDIO_UART_DEV
 # endif /* MODULE_SLIPDEV_STDIO */
 #endif  /* SLIPDEV_PARAM_UART */
 #ifndef SLIPDEV_PARAM_BAUDRATE

diff --git a/makefiles/tests/tests.inc.mk b/makefiles/tests/tests.inc.mk
@@ -85,17 +85,43 @@ test-with-config/check-config:
 		done; \
 		${COLOR_ECHO} -n "${COLOR_RESET}"
 
+
+RIOT_TEST_RIOT_TEST_HASH_DIR ?= $(BINDIR)
+
 # this target only makes sense if an ELFFILE is actually created, thus guard by
 # RIOTNOLINK="".
 ifeq (,$(RIOTNOLINK))
   ifeq (,$(HASHFILE))
     $(error HASHFILE is empty for $(BOARD))
   endif
 test-input-hash: $(TESTS) $(TESTS_WITH_CONFIG) $(TESTS_AS_ROOT) $(HASHFILE) $(TEST_EXTRA_FILES)
-	sha1sum $^ > $(BINDIR)/test-input-hash.sha1
+	sha1sum $^ > $(RIOT_TEST_HASH_DIR)/test-input-hash.sha1
 else
 # .SECONDARY creates the bin folder, we depend on it to avoid writing to it
 # prior to it being created when concurrent building is used
 test-input-hash: .SECONDARY
-	$(file >$(BINDIR)/test-input-hash.sha1,no binary generated due to RIOTNOLINK=1)
+	$(file >$(RIOT_TEST_HASH_DIR)/test-input-hash.sha1,no binary generated due to RIOTNOLINK=1)
 endif
+
+# Helper function to compare two strings
+define compare_strings
+$(and $(filter $1,$2),$(filter $2,$1))
+endef
+
+# Target to test only if the input hash has changed
+.PHONY: test-input-hash-changed
+test-input-hash-changed:
+	@if [ ! -f "$(RIOT_TEST_HASH_DIR)/test-input-hash.sha1" ]; then \
+		echo "Old hash file doesn't exist. Generating hash and running tests..."; \
+		mkdir -p $(RIOT_TEST_HASH_DIR); \
+		$(MAKE) test-input-hash; \
+	else \
+		OLD_HASH=$$(cat $(RIOT_TEST_HASH_DIR)/test-input-hash.sha1); \
+		$(MAKE) test-input-hash; \
+		NEW_HASH=$$(cat $(RIOT_TEST_HASH_DIR)/test-input-hash.sha1); \
+		if [ "$${OLD_HASH}" != "$${NEW_HASH}" ]; then \
+			echo "Hashes do not match."; \
+		else \
+			echo "Hashes match."; \
+		fi; \
+	fi
diff --git a/makefiles/vars.inc.mk b/makefiles/vars.inc.mk
@@ -46,6 +46,7 @@ export RIOTMAKE              # Location of all supplemental Makefiles (such as t
 export RIOTKCONFIG           # Location of all supplemental Kconfig files
 export BINDIRBASE            # This is the folder where the application should be built in. For each BOARD a different subfolder is used.
 export BINDIR                # This is the folder where the application should be built in.
+export RIOT_TEST_HASH_DIR    # The dir to generate the test-input-hash.sha1 file for checking if a test has changed, uses BINDIR by default.
 export CARGO_TARGET_DIR      # This is the folder where Rust parts of the application should be built in.
 export BUILD_DIR             # This is the base folder to store common build files and artifacts, e.g. test results.
 export APPDIR                # The base folder containing the application

diff --git a/pkg/lwip/contrib/netdev/lwip_netdev.c b/pkg/lwip/contrib/netdev/lwip_netdev.c
@@ -62,6 +62,15 @@ static err_t _eth_link_output(struct netif *netif, struct pbuf *p);
 #ifdef MODULE_LWIP_SIXLOWPAN
 static err_t _ieee802154_link_output(struct netif *netif, struct pbuf *p);
 #endif
+#ifdef MODULE_SLIPDEV
+static err_t _slip_link_output(struct netif *netif, struct pbuf *p);
+#if LWIP_IPV4
+static err_t slip_output4(struct netif *netif, struct pbuf *q, const ip4_addr_t *ipaddr);
+#endif
+#if LWIP_IPV6
+static err_t slip_output6(struct netif *netif, struct pbuf *q, const ip6_addr_t *ip6addr);
+#endif
+#endif
 static void _event_cb(netdev_t *dev, netdev_event_t event);
 static void *_event_loop(void *arg);
 
@@ -183,6 +192,47 @@ err_t lwip_netdev_init(struct netif *netif)
 #endif /* LWIP_IPV6_AUTOCONFIG */
         break;
     }
+#endif
+#ifdef MODULE_SLIPDEV
+    case NETDEV_TYPE_SLIP:
+        netif->name[0] = 'S';
+        netif->name[1] = 'L';
+
+        /* TODO: get from driver (currently not in netdev_eth) */
+        netif->mtu = ETHERNET_DATA_LEN;
+        netif->linkoutput = _slip_link_output;
+#if LWIP_IPV4
+        netif->output = slip_output4;
+#endif
+#if LWIP_IPV6
+        netif->output_ip6 = slip_output6;
+
+        if (IS_USED(MODULE_SLIPDEV_L2ADDR)) {
+            netif->hwaddr_len = (u8_t)netdev->driver->get(netdev, NETOPT_ADDRESS_LONG,
+                                                          netif->hwaddr,
+                                                          sizeof(netif->hwaddr));
+            if (netif->hwaddr_len > sizeof(netif->hwaddr)) {
+                res = ERR_IF;
+                goto free;
+            }
+
+            /* netif_create_ip6_linklocal_address() does weird byte-swapping
+             * with full IIDs, so let's do it ourselves */
+            ip6_addr_t *addr = ip_2_ip6(&(netif->ip6_addr[0]));
+            /* addr->addr is a uint32_t array */
+            if (l2util_ipv6_iid_from_addr(dev_type,
+                                          netif->hwaddr, netif->hwaddr_len,
+                                          (eui64_t *)&addr->addr[2]) < 0) {
+                res = ERR_IF;
+                goto free;
+            }
+            ipv6_addr_set_link_local_prefix((ipv6_addr_t *)&addr->addr[0]);
+            ip6_addr_assign_zone(addr, IP6_UNICAST, netif);
+            /* Consider address valid. */
+            netif->ip6_addr_state[0] = IP6_ADDR_PREFERRED;
+        }
+#endif /* LWIP_IPV6 */
+        break;
 #endif
     default:
         res = ERR_IF;
@@ -257,6 +307,38 @@ static err_t _ieee802154_link_output(struct netif *netif, struct pbuf *p)
 }
 #endif
 
+#ifdef MODULE_SLIPDEV
+#if LWIP_IPV4
+static err_t slip_output4(struct netif *netif, struct pbuf *q, const ip4_addr_t *ipaddr)
+{
+    (void)ipaddr;
+    return netif->linkoutput(netif, q);
+}
+#endif
+#if LWIP_IPV6
+static err_t slip_output6(struct netif *netif, struct pbuf *q, const ip6_addr_t *ip6addr)
+{
+    (void)ip6addr;
+    return netif->linkoutput(netif, q);
+}
+#endif
+
+static err_t _slip_link_output(struct netif *netif, struct pbuf *p)
+{
+    LWIP_ASSERT("p->next == NULL", p->next == NULL);
+    netdev_t *netdev = netif->state;
+    iolist_t pkt = {
+        .iol_base = p->payload,
+        .iol_len = p->len,
+    };
+
+    lwip_netif_dev_acquire(netif);
+    err_t res = (netdev->driver->send(netdev, &pkt) >= 0) ? ERR_OK : ERR_BUF;
+    lwip_netif_dev_release(netif);
+    return res;
+}
+#endif
+
 static struct pbuf *_get_recv_pkt(netdev_t *dev)
 {
     lwip_netif_t *compat_netif = dev->context;

diff --git a/pkg/lwip/init_devs/auto_init_slipdev.c b/pkg/lwip/init_devs/auto_init_slipdev.c
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2023 ML!PA Consulting GmbH
+ *
+ * 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.
+ */
+
+/**
+ * @ingroup sys_auto_init_lwip_netif
+ * @{
+ *
+ * @file
+ * @brief   Auto initialization for the SLIP module
+ *
+ * @author  Benjamin Valentin <[email protected]>
+ */
+
+#include "slipdev.h"
+#include "slipdev_params.h"
+
+#include "lwip_init_devs.h"
+
+#define ENABLE_DEBUG    0
+#include "debug.h"
+
+#define NETIF_SLIPDEV_NUMOF ARRAY_SIZE(slipdev_params)
+
+static lwip_netif_t netif[NETIF_SLIPDEV_NUMOF];
+static slipdev_t slipdev_devs[NETIF_SLIPDEV_NUMOF];
+
+static void auto_init_slipdev(void)
+{
+    for (unsigned i = 0; i < NETIF_SLIPDEV_NUMOF; i++) {
+        slipdev_setup(&slipdev_devs[i], &slipdev_params[i], i);
+        if (lwip_add_ethernet(&netif[i], &slipdev_devs[i].netdev) == NULL) {
+            DEBUG("Could not add slipdev device\n");
+            return;
+        }
+    }
+}
+
+LWIP_INIT_ETH_NETIF(auto_init_slipdev);
+/** @} */
diff --git a/tests/drivers/at/main.c b/tests/drivers/at/main.c
@@ -47,6 +47,11 @@ static int init(int argc, char **argv)
     uint8_t uart = atoi(argv[1]);
     uint32_t baudrate = atoi(argv[2]);
 
+    if (uart >= UART_NUMOF) {
+        printf("Wrong UART device number - should be in range 0-%d.\n", UART_NUMOF - 1);
+        return 1;
+    }
+
     int res = at_dev_init(&at_dev, UART_DEV(uart), baudrate, buf, sizeof(buf));
 
     /* check the UART initialization return value and respond as needed */