[riot-notifications] [RIOT-OS/RIOT] dist/tools/renode: improve integration + additional boards (#15459)

Alexandre Abadie notifications at github.com
Tue Nov 17 21:15:07 CET 2020


@aabadie commented on this pull request.

Having a good support for renode would be super, so thanks for the effort @basilfx, @Citrullin.

I have 2 comments about this PR (see below): one is about a possible improvement with the build system and the other one is about the addition of all these .repl files that are also present upstream. I think we should avoid that and try to use the ones that comes from the upstream project, and eventually contribute there if some are missing, instead of putting everything in the RIOT codebase: this will minimize our maintenance burden but also will increase potential collaborations between RIOT and the renode project.

> @@ -0,0 +1,25 @@
+:name: STK3700
+:description: Silicon Labs STK3700 starter kit
+
+$name?="STK3700"
+
+using sysbus
+mach create $name
+
+machine LoadPlatformDescription $RIOTCPU/efm32/families/efm32gg/dist/renode/efm32gg990f1024.repl
+machine LoadPlatformDescription $RIOTBOARD/stk3700/dist/renode/stk3700.repl

Why not use [the repl provided by the upstream project](https://github.com/renode/renode/blob/master/platforms/boards/silabs/stk3700.repl) ? That would avoid the addition of a lot of repl I think.

> +# setup emulator
+include $(RIOTMAKE)/tools/renode.inc.mk

This could be included globally in `$(RIOTBASE)/Makefile.include` if the `emulate` target is part of the `MAKECMDGOALS` variable. This would avoid the need to duplicate this line in all boards with the emulate support.

```diff
diff --git a/Makefile.include b/Makefile.include
index 350d15fd08..bc749ffe8a 100644
--- a/Makefile.include
+++ b/Makefile.include
@@ -389,6 +389,11 @@ else
   include $(RIOTMAKE)/dependency_resolution.inc.mk
 endif
 
+# Include renode emulator variables if the emulate target is used
+ifneq (,$(filter emulate,$(MAKECMDGOALS)))
+  include $(RIOTMAKE)/tools/renode.inc.mk
+endif
+
 # Include Board and CPU configuration
 INCLUDES += $(addprefix -I,$(wildcard $(BOARDDIR)/include))
 include $(BOARDDIR)/Makefile.include
```

It might even be possible to check if the board provides the renode feature at that level.

-- 
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/15459#pullrequestreview-532761966
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20201117/9a68ee37/attachment.htm>


More information about the notifications mailing list