[TRACKING] buildsystem: introduce "make rawterm" #11099
Conversation
jcarrano
added
Type: bug
|
What's wrong with using picocom (or socat) for tests? |
|
Current Also, when using |
|
Also, there are already cases where you cannot directly say "RIOT_TERMINAL = picocom" and expect it to work. By putting a name on it, it allows doing test specific configurations without breaking the |
Nothing, quite the contrary, it is GOOD.
There are not many ways to escape issue (3). |
|
To introduce it early without even being implemented, I would like to have an implementation of With a documentation saying the characteristics of This way, it could be used in the tests by default. As the first thing would be that it does not change the current behavior, just the target being used. If we manage to have all boards supported, the "backup" could even be removed at the end. |
jcarrano
added
CI: ready for build
|
I just ran the tests on a SAMR21 on my machine. Seems to work. I'd like to try a board with jlink rtt. |
jcarrano
added
CI: ready for build
jcarrano
added
the
CI: ready for build
|
I rebased on top of master. |
`make rawterm` is the new interface for automated access to the serial port. It will be used in testing. In contrast with `make term`, which is intended for human-machine-interaction, rawterm is meant to comminicate the node with automated scripts running on the host machine. rawterm must: - Not have any input or output buffering. - Not modify input or output in any way (no extra prompts, logging artifacts, translations etc.) - Have local echo disabled. - Not print anything to stdout that was not generated by the node (i.e. no startup banners.) With this definition it is very easy to implement a generic "nice" terminal (that is `make term`) as a wrapper around rawterm.
Add socat, picocom and jlink (for rtt) as terminals, and remove socat from the cooked terminal list.
Rlwrap allows using any raw terminal (if it is really raw) and adding readline support around it, with history that is even preserved thoughout sessions. The -C option is used to create a different history file for each board.
The lobaro-lorabox sets some pyterm-specific TERMFLAGS, so we must make sure that pyterm is selected.
The msba2 sets some pyterm-specific TERMFLAGS, so we must make sure that pyterm is selected.
The riot executable is NOT a terminal program. Therefore, to mantain consistency with the rest of the targets and avoid hacky-hacks the TERMPROG should not be ELFFILE. This commit creates a new variables specially for the native platform, NATIVE_EXEC, NATIVE_FLAGS. The (raw)terminal program is socat, which wraps NATIVE_EXEC. This also has the benefit of disabling buffering and echo in the terminal so that narive behaves more similar to bare-metal RIOT. Doing this terminal reconfiguration in RIOT is possible, but dangerous as the settings must be reset after exit or the users terminal emulator will be left broken. Using socat as RIOT_RAWTERMINAL means rlwrap can be used as RIOT_TERMINAL, further unifying the behaviour of native and bare-metal.
The native executable is no longer the TERMPROG. Therefore, the flags are not given by TERMFLAGS, but by NATIVE_FLAGS.
cladmi
added
the
State: waiting for other PR
| @@ -19,6 +19,8 @@ PORT_LINUX ?= /dev/ttyUSB0 | |||
| # This does not make a lot of sense, but it has the same value as the previous code | |||
| PORT_DARWIN ?= /dev/tty.usbserial-ARM | |||
|
|
|||
| # The -tg option is specific to pyterm so we are forced to use it. | |||
| RIOT_TERMINAL = pyterm | |||
There was a problem hiding this comment.
Can't you also use simply expanded variables := here and in all following cases below? There's no need for recursions as it appears to me.
There was a problem hiding this comment.
At the moment we are trying to do the opposite: remove instances of := where it is not needed.
There was a problem hiding this comment.
Interesting .. as far as I know you usually remove instances of =, where it is not needed (: maintaining recursively expanded variables is complex and can lead to hard-to-debug problems. Imagine someone replaces pyterm with a function or a variable in the future, but keeps the = ..
What's the motivation for choosing recursive assignments over simple assignments?
There was a problem hiding this comment.
:= has some disadvantages:
- The value of variables defined with
:=depends on the order they are defined. - Once introduced, it is not that easy to tell if
:=can be replaced by= :=will be evaluated even if not needed. This will force evaluation of all called functions, in particular shell calls. This causes unnecessary overhead at minimum, or a failure that should not occur at most (think what happens if a tool is defined with:=but you don't have the tool and you don't need it either)
I believe immediate assignments cause harder-to-debug problems because the order of definition starts to matter.
With respect to performance, if some variable is too expensive to evaluate more than once, there is a trick to memoize it (I think there is a PR by @cladmi ).
Finally, recursive variables fit better into the declarative style that good makefiles should follow.
There was a problem hiding this comment.
a quote from the docs (:
To avoid all the problems and inconveniences of recursively expanded variables,
there is another flavor: simply expanded variables.
Kidding aside, I am not very involved in the build system development and I don't know what the desired end result will look like. I was merely stating that I had problems with recursive evaluations in earlier projects and most of the time they lead to performance drops. If you think that recursive evaluations fit better in the picture, then I trust your judgement.
There was a problem hiding this comment.
The reality is that we (and by that I mean @cladmi and me) should probably spend some time writing a proper guide to writing makefiles in RIOT.
That part of the docs is true, but it refers mainly to things like wildcards and shell calls with side-efects. Essentially, I/O. In our case := is most of the times workaround for this. The real solution is to more cleanly separate the functional and I/O parts of our makefiles.
Performance drop in RIOT builds is mainly due to:
- FORCE and PHONY targets (for example, we are relinking every time).
- these are ugly hacks because people could not figure out how to do dependencies properly.
- Recursive make (or more precisely, the fact that the submakes run always)
- Checking tools all the time / evaluating tool-related variables, even when those tools are not used (
:=is involved here).- Here is where a memoization function will help, but only after we refactor.
This is probably more info than you asked for, but I feel this should be stated somewhere.
There was a problem hiding this comment.
If you are doing recursive definitions you are not defining but doing evaluation.
Here I would say there is no need for evaluation so no :=.
Interesting .. as far as I know you usually remove instances of =, where it is not needed (: maintaining recursively expanded variables is complex and can lead to hard-to-debug problems. Imagine someone replaces pyterm with a function or a variable in the future, but keeps the = ..
usually I would say no, its only in RIOT that I saw this pattern.
If you have INCLUDES that depends on INCLUDES you should introduce a new name and do INCLUDES = $(INCLUDES_TOOLCHAIN) $(INCLUDES_C_LIBRARY) and define INCLUDES_TOOLCHAIN later instead of prepending to the previous value.
If you replace pyterm with a function, it does nothing as you are not evaluating the name with $(pyterm).
I was merely stating that I had problems with recursive evaluations in earlier projects and most of the time they lead to performance drops.
If a shell call is evaluated 100 times it can indeed slow down the build, but then only this one should be optimized. Also, with correct file targets you do not rebuild the target if not neheaderseded so do not re-evaluate the variable for nothing. It may make the clean build slower but makes incremental build faster.
Also, by doing := everywhere you are making make clean slow as it checks the warnings supported for your gcc version even if you do not use it.
There was a problem hiding this comment.
It is somehow "gnumake" basics, but how it should be applied to RIOT could indeed be good.
I never had any issues before as was not using recursive makefiles.
We still need to export things and so how to counter the issues with it with memoized somehow and target-export-variables.
|
@cladmi Do you still have the test we did with socat inside jlink.sh (with the automatic retry and all that stuff)? |
|
A hardwritten sleep was added RIOT/dist/tools/jlink/jlink.sh Line 258 in fcd7f22 For the retry I had this #10849 but was not using |
|
|
||
| RIOT_RAWTERMINAL ?= socat | ||
| ifeq ($(RIOT_RAWTERMINAL),socat) | ||
| SOCAT_LOCAL ?= STDIO,icanon=0,echo=0,escape=0x04 |
There was a problem hiding this comment.
With these options I'm still getting line buffering.
|
#12107 was merged. |
|
This is not finished, line buffering is still not addressed here with pyterm. |
|
Ah sorry, overlooked that. |
Maybe I can cherry-pick some of these... I'll add milestone to check later... |
|
@fjmolinas, did you check? ;-) |
Nope, and I don't think I'll have time, lets postpone to 2022.07 |
|
Stale and apparently no longer needed |
Contribution description
This is a tracking issue. It will be split.
Currently, the tests invoke "make term" for interacting with the board. "make term" starts the "pyterm" program which may be nice for a human user but is horrible for a machine: it does line buffering, local echo, inserts lots of unnecessary data in the output stream, has "magical" behaviors (like hitting enter on an empty line repeats the last command, wtf?!?!). Normally, when testing, you want to test the actual thing and try to take out of the equation any external factors. In any case, it's pointless to use any readline-based terminal program when the output is not a pty/tty but a pipe.
Add to this that the tests are done using "dumb matching" with pexpect and the result is that you try to match, say, an integer, and you could be matching your own input, or pyterm's "logging" info.
We are introducing "make rawterm" which gives a "raw" terminal, meaning one that is transparent- whatever you give to the terminal input goes to the serial device, whatever arrives to the serial device- and nothing- else goes to the terminal output.
On the native side, I tried to fix the conceptual error of having TERMPROG be the riot executable and TERMFLAGS be the flags to RIOT.
Other facts to take into account:
Testing procedure
I will provide some tests stolen from #11094 to show the issues with the non-raw terminals and how they interfere with testing.
TODO:
Issues/PRs references
Contains commits from: #11156, #11129 (needed for tests)
Would fix: #10952
Part of: #10994
Related to: #11094