sys/shell: convert shell_commands.c commands to use XFA #16095

kaspar030 · 2021-02-25T12:05:11Z

Contribution description

This PR changes shell_commands.c to use the new XFA method instead of a seperate array for defining "builtin" shell commands.
It is the first step towards cleaning up the ifdefs by moving each shell command to its respective module sources.

Testing procedure

This touches basically every application using the shell, and might thus expose issues with XFA. Running any shell based tests on diverse platforms makes sense.

Issues/PRs references

Waiting for ~~#16094~~, ~~#16061~~

kaspar030 · 2021-02-25T22:30:16Z

Doh,

In file included from shell_commands.c:25:0:
shell_commands.c:225:18: error: expected â€˜=â€™, â€˜,â€™, â€˜;â€™, â€˜asmâ€™ or â€˜__attribute__â€™ before â€˜-â€™ token
 SHELL_COMMAND(sht-config, "Get/set SHT10/11/15 sensor configuration.", _sht_config_handler);
                  ^
../../include/shell.h:230:37: note: in definition of macro â€˜SHELL_COMMANDâ€™
     static shell_command_t _xfa_ ## name ## _cmd = { #name, help, &func }; \

(I somehow assumed that shell command names could become symbol names)

miri64 · 2021-02-25T22:33:48Z

(I somehow assumed that shell command names could become symbol names)

Another reason why auto-generating shell handler names might not be desirable. I personally prefer hyphened commands over underdashed commands. They seem subjectively less "clunky".

sys/shell/commands/shell_commands.c

sys/include/shell.h

sys/shell/commands/shell_commands.c

kaspar030 · 2021-03-02T10:23:02Z

Another reason why auto-generating shell handler names might not be desirable. I personally prefer hyphened commands over underdashed commands. They seem subjectively less "clunky".

I found two hyphened shell commands in-tree (sht-config and send-test-pkt), versus dozens of underdashed ones. For consistency alone those hyphens should be changed.

Having the shell command name adhere to C symbol naming rules has the huge advantage of being able to let the linker sort them alphanumerically.

I'd like to go without hyphens / C symbol nameing rules for now.

miri64 · 2021-03-02T10:29:05Z

Having the shell command name adhere to C symbol naming rules has the huge advantage of being able to let the linker sort them alphanumerically.

The value of a string has influence on the linker's sorting order? I somehow doubt that. The variable names do not change with my proposal.

miri64 · 2021-03-02T10:30:28Z

I found two hyphened shell commands in-tree (sht-config and send-test-pkt), versus dozens of underdashed ones. For consistency alone those hyphens should be changed.

I would prefer on the other hand, that characters beyond A-Za-z0-9_ were allowed for command names, because there is no real reason to restrict them.

miri64 · 2021-03-02T10:32:18Z

I would prefer on the other hand, that characters beyond A-Za-z0-9_ were allowed for command names, because there is no real reason to restrict them.

It can also quickly lead to inconsistencies since app shell commands (which I assume will still use the old style of command declaration) will be allowed to have those characters (since they are not checked by the compile), while build-in commands are not allowed to have those characters.

kaspar030 · 2021-03-02T10:34:38Z

@miri64 your proposal seems doable. For now I changed the only builtin command (sht-config -> sht_config). Let's revisit this in a follow up, ok?

miri64 · 2021-03-02T10:35:43Z

@miri64 your proposal seems doable. For now I changed the only builtin command (sht-config -> sht_config). Let's revisit this in a follow up, ok?

Ok.

miri64 · 2021-03-02T10:36:00Z

Unless tests fail due to this ;-).

kaspar030 · 2021-03-02T10:46:52Z

rebased to get congure shell commands in

kaspar030 · 2021-03-02T10:49:37Z

Unless tests fail due to this ;-).

git grep 'sht-config' is empty in tests, so I'm positive. That driver doesn't seem to have a test application.

kaspar030 · 2021-03-02T10:55:04Z

The value of a string has influence on the linker's sorting order? I somehow doubt that. The variable names do not change with my proposal.

no, without your proposal, XFA's get sorted by symbol name. If they're equal to the string representation of a command, this sorts shell commands in the same order.

miri64 · 2021-03-02T11:05:12Z

Unless tests fail due to this ;-).

git grep 'sht-config' is empty in tests, so I'm positive. That driver doesn't seem to have a test application.

Let's hope it does not fall on our foot elsewhere ... ;-)

kaspar030 · 2021-03-02T11:16:26Z

Let's hope it does not fall on our foot elsewhere ... ;-)

what do you have in mind?

miri64 · 2021-03-02T11:38:54Z

Let's hope it does not fall on our foot elsewhere ... ;-)

what do you have in mind?

I just had an experience, where a simple addition let to explosions elsewhere where I was not anticipating it...

miri64 · 2021-03-09T13:49:49Z

Needs rebase now.

This reverts commit d9cda53.

kaspar030 · 2021-03-09T14:04:11Z

rebased with sys/shell: add XFA support #16061 in, not waiting anymore

miri64 · 2021-03-09T15:17:31Z

LGTM now. I started a release test run on this PR to get some coverage on the shell commands. Would be great if @fjmolinas could also run TRIBE on this. I wait for my final ACK until some order of operation regarding #16135 (comment) was figured out.

miri64 · 2021-03-09T15:33:38Z

Would be great if @fjmolinas could also run TRIBE on this.

Until then, I also started a run on the testbed

miri64 · 2021-03-10T10:32:08Z

I started a release test run on this PR to get some coverage on the shell commands.

Failed for unrelated reasons in task 1.2 and 1.3 (also occured during the latest weekly)

01-ci.test_spec01

Test case:	test_task02[nodes0]
Outcome:	Failed
Duration:	146.3 sec
Failed	subprocess.CalledProcessError: Command '['make', '--no-print-directory', '-C', '/home/runner/work/RIOT/RIOT/RIOT/tests/unittests', 'all', 'flash-only', 'test']' returned non-zero exit status 2.

nodes = [<riotctrl.ctrl.RIOTCtrl object at 0x7faa55d576d0>], log_nodes = True
riotbase = '/home/runner/work/RIOT/RIOT/RIOT'
@pytest.mark.parametrize('nodes',
                         [pytest.param(['native'])],
                         indirect=['nodes'])
def test_task02(nodes, log_nodes, riotbase):

>       run_unittests('all', nodes, log_nodes, os.path.join(riotbase, APP))
01-ci/test_spec01.py:31:

01-ci/test_spec01.py:19: in run_unittests

node.make_run(

.tox/test/lib/python3.9/site-packages/riotctrl/ctrl.py:178: in make_run

return subprocess.run(command, env=self.env, *runargs, **runkwargs)

input = None, capture_output = False, timeout = None, check = True

popenargs = (['make', '--no-print-directory', '-C', '/home/runner/work/RIOT/RIOT/RIOT/tests/unittests', 'all', 'flash-only', ...],)

kwargs = {'env': {'BOARD': 'native', 'BUILD_IN_DOCKER': '1', 'DOCKER_ENV_VARS': 'USEMODULE', 'DOCKER_MAKE_ARGS': '-j', ...}, 'stderr': None, 'stdout': None}

process = <Popen: returncode: 2 args: ['make', '--no-print-directory', '-C', '/home/ru...>

stdout = None, stderr = None, retcode = 2
def run(*popenargs,
        input=None, capture_output=False, timeout=None, check=False, **kwargs):
    """Run command with arguments and return a CompletedProcess instance.

    The returned instance will have attributes args, returncode, stdout and
    stderr. By default, stdout and stderr are not captured, and those attributes
    will be None. Pass stdout=PIPE and/or stderr=PIPE in order to capture them.

    If check is True and the exit code was non-zero, it raises a
    CalledProcessError. The CalledProcessError object will have the return code
    in the returncode attribute, and output &amp; stderr attributes if those streams
    were captured.

    If timeout is given, and the process takes too long, a TimeoutExpired
    exception will be raised.

    There is an optional argument "input", allowing you to
    pass bytes or a string to the subprocess's stdin.  If you use this argument
    you may not also use the Popen constructor's "stdin" argument, as
    it will be used internally.

    By default, all communication is in bytes, and therefore any "input" should
    be bytes, and the stdout and stderr will be bytes. If in text mode, any
    "input" should be a string, and stdout and stderr will be strings decoded
    according to locale encoding, or by "encoding" if set. Text mode is
    triggered by setting any of text, encoding, errors or universal_newlines.

    The other arguments are the same as for the Popen constructor.
    """
    if input is not None:
        if kwargs.get('stdin') is not None:
            raise ValueError('stdin and input arguments may not both be used.')
        kwargs['stdin'] = PIPE

    if capture_output:
        if kwargs.get('stdout') is not None or kwargs.get('stderr') is not None:
            raise ValueError('stdout and stderr arguments may not be used '
                             'with capture_output.')
        kwargs['stdout'] = PIPE
        kwargs['stderr'] = PIPE

    with Popen(*popenargs, **kwargs) as process:
        try:
            stdout, stderr = process.communicate(input, timeout=timeout)
        except TimeoutExpired as exc:
            process.kill()
            if _mswindows:
                # Windows accumulates the output in a single blocking
                # read() call run on child threads, with the timeout
                # being done in a join() on those threads.  communicate()
                # _after_ kill() is required to collect that and add it
                # to the exception.
                exc.stdout, exc.stderr = process.communicate()
            else:
                # POSIX _communicate already populated the output so
                # far into the TimeoutExpired exception.
                process.wait()
            raise
        except:  # Including KeyboardInterrupt, communicate handled that.
            process.kill()
            # We don't call process.wait() as .__exit__ does that for us.
            raise
        retcode = process.poll()
        if check and retcode:

>               raise CalledProcessError(retcode, process.args,

output=stdout, stderr=stderr)

E               subprocess.CalledProcessError: Command '['make', '--no-print-directory', '-C', '/home/runner/work/RIOT/RIOT/RIOT/tests/unittests', 'all', 'flash-only', 'test']' returned non-zero exit status 2.
/opt/hostedtoolcache/Python/3.9.2/x64/lib/python3.9/subprocess.py:528: CalledProcessError

Stdout

--------------------------------- Captured Log ---------------------------------

--------------------------------- Captured Out ---------------------------------
Launching build container using image "riot/riotbuild:latest".
docker run --rm --tty --user $(id -u) -v '/usr/share/zoneinfo/Etc/UTC:/etc/localtime:ro' -v '/home/runner/work/RIOT/RIOT/RIOT:/data/riotbuild/riotbase:delegated' -e 'RIOTBASE=/data/riotbuild/riotbase' -e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' -e 'BUILD_DIR=/data/riotbuild/riotbase/build' -e 'RIOTPROJECT=/data/riotbuild/riotbase' -e 'RIOTCPU=/data/riotbuild/riotbase/cpu' -e 'RIOTBOARD=/data/riotbuild/riotbase/boards' -e 'RIOTMAKE=/data/riotbuild/riotbase/makefiles' -e 'BOARD=native' -w '/data/riotbuild/riotbase/tests/unittests/' 'riot/riotbuild:latest' make -j all
#x1B[1;32mBuilding application "tests_unittests" for "native" with MCU "native".#x1B[0m

"make" -C /data/riotbuild/riotbase/boards/native
"make" -C /data/riotbuild/riotbase/core
"make" -C /data/riotbuild/riotbase/cpu/native
"make" -C /data/riotbuild/riotbase/drivers
"make" -C /data/riotbuild/riotbase/sys
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-analog_util
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-base64
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-bcd
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-bitfield
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-bloom
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-bluetil
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-checksum
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-clif
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-color
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-core
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-credman
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-div
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-ecc
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-fib
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-fib_sr
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-fmt
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-frac
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-gcoap
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-gnrc_ipv6
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-gnrc_ipv6_hdr
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-gnrc_ipv6_nib
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-gnrc_mac_internal
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-gnrc_netif_pktq
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-gnrc_sixlowpan_frag_vrb
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-gnrc_udp
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-hashes
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-ieee802154
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-inet_csum
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-ipv4_addr
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-ipv6_addr
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-ipv6_hdr
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-luid
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-matstat
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-mtd
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-nanocoap
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-netopt
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-netreg
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-phydat
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-pkt
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-pktbuf
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-pktqueue
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-printf_float
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-priority_pktqueue
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-rtc
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-rtt_rtc
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-saul_reg
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-scanf_float
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-seq
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-sht1x
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-sixlowpan
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-sixlowpan_ctx
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-sixlowpan_sfr
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-sock_util
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-timex
"make" -C /data/riotbuild/riotbase/cpu/native/mtd
"make" -C /data/riotbuild/riotbase/cpu/native/periph
"make" -C /data/riotbuild/riotbase/cpu/native/stdio_native
"make" -C /data/riotbuild/riotbase/cpu/native/vfs
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-tsrb
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-uri_parser
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-uuid
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-vfs
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-zptr
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-ztimer
"make" -C /data/riotbuild/riotbase/sys/analog_util
"make" -C /data/riotbuild/riotbase/sys/base64
"make" -C /data/riotbuild/riotbase/sys/bitfield
"make" -C /data/riotbuild/riotbase/sys/bloom
"make" -C /data/riotbuild/riotbase/sys/checksum
"make" -C /data/riotbuild/riotbase/sys/clif
"make" -C /data/riotbuild/riotbase/sys/color
"make" -C /data/riotbuild/riotbase/sys/crypto
"make" -C /data/riotbuild/riotbase/sys/div
"make" -C /data/riotbuild/riotbase/sys/ecc
"make" -C /data/riotbuild/riotbase/sys/embunit
"make" -C /data/riotbuild/riotbase/sys/event
"make" -C /data/riotbuild/riotbase/sys/evtimer
"make" -C /data/riotbuild/riotbase/sys/fmt
"make" -C /data/riotbuild/riotbase/boards/native/drivers
"make" -C /data/riotbuild/riotbase/sys/frac
"make" -C /data/riotbuild/riotbase/sys/fs/constfs
"make" -C /data/riotbuild/riotbase/sys/hashes
"make" -C /data/riotbuild/riotbase/drivers/mtd
"make" -C /data/riotbuild/riotbase/drivers/periph_common
"make" -C /data/riotbuild/riotbase/drivers/rtt_rtc
"make" -C /data/riotbuild/riotbase/drivers/saul
"make" -C /data/riotbuild/riotbase/drivers/sht1x
"make" -C /data/riotbuild/riotbase/sys/luid
"make" -C /data/riotbuild/riotbase/sys/matstat
"make" -C /data/riotbuild/riotbase/sys/net/application_layer/gcoap
"make" -C /data/riotbuild/riotbase/sys/net/application_layer/nanocoap
"make" -C /data/riotbuild/riotbase/sys/net/ble/bluetil
"make" -C /data/riotbuild/riotbase/sys/net/credman
"make" -C /data/riotbuild/riotbase/sys/net/crosslayer/inet_csum
"make" -C /data/riotbuild/riotbase/sys/net/crosslayer/netopt
"make" -C /data/riotbuild/riotbase/sys/net/gnrc
"make" -C /data/riotbuild/riotbase/sys/net/link_layer/csma_sender
"make" -C /data/riotbuild/riotbase/sys/net/link_layer/ieee802154
"make" -C /data/riotbuild/riotbase/sys/net/link_layer/l2util
"make" -C /data/riotbuild/riotbase/sys/net/netif
"make" -C /data/riotbuild/riotbase/sys/net/network_layer/fib
"make" -C /data/riotbuild/riotbase/sys/net/network_layer/icmpv6
"make" -C /data/riotbuild/riotbase/sys/net/network_layer/ipv4/addr
"make" -C /data/riotbuild/riotbase/sys/net/network_layer/ipv6/addr
"make" -C /data/riotbuild/riotbase/sys/net/network_layer/ipv6/hdr
"make" -C /data/riotbuild/riotbase/sys/net/network_layer/sixlowpan
"make" -C /data/riotbuild/riotbase/sys/net/sock
"make" -C /data/riotbuild/riotbase/sys/net/sock/async/event
"make" -C /data/riotbuild/riotbase/sys/net/transport_layer/udp
"make" -C /data/riotbuild/riotbase/sys/od
"make" -C /data/riotbuild/riotbase/sys/phydat
"make" -C /data/riotbuild/riotbase/sys/posix/inet
"make" -C /data/riotbuild/riotbase/sys/random
"make" -C /data/riotbuild/riotbase/sys/saul_reg
"make" -C /data/riotbuild/riotbase/sys/seq
"make" -C /data/riotbuild/riotbase/sys/test_utils/interactive_sync
"make" -C /data/riotbuild/riotbase/sys/timex
"make" -C /data/riotbuild/riotbase/sys/tsrb
"make" -C /data/riotbuild/riotbase/sys/universal_address
"make" -C /data/riotbuild/riotbase/sys/uri_parser
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/link_layer/mac
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/netapi
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/netif
"make" -C /data/riotbuild/riotbase/sys/uuid
"make" -C /data/riotbuild/riotbase/sys/vfs
"make" -C /data/riotbuild/riotbase/sys/xtimer
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/netreg
"make" -C /data/riotbuild/riotbase/sys/net/ble/bluetil/bluetil_addr
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/network_layer/icmpv6
"make" -C /data/riotbuild/riotbase/sys/ztimer
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/network_layer/ipv6
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/network_layer/ipv6/hdr
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/network_layer/ipv6/nib
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/network_layer/ndp
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/network_layer/sixlowpan
"make" -C /data/riotbuild/riotbase/sys/random/tinymt32
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/network_layer/sixlowpan/ctx
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/network_layer/sixlowpan/frag/fb
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/network_layer/sixlowpan/frag/vrb
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/network_layer/sixlowpan/nd
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/pkt
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/pktbuf
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/pktbuf_static
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/priority_pktqueue
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/sock
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/sock/udp
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/netif/hdr
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/netif/pktq
"make" -C /data/riotbuild/riotbase/sys/net/gnrc/transport_layer/udp
text data bss dec hex filename
862541 137454 80004 1079999 107abf /data/riotbuild/riotbase/tests/unittests/bin/native/tests_unittests.elf
r
/home/runner/work/RIOT/RIOT/RIOT/tests/unittests/bin/native/tests_unittests.elf /dev/ttyACM0
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: cb25-HEAD)
Help: Press s to start test, r to print it is ready
READY
s
START
.........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
rtc_tests.test_rtc_compat (tests/unittests/tests-rtc/tests-rtc.c 29) exp 0 was 6
.
rtc_tests.test_rtc_sec_wrap (tests/unittests/tests-rtc/tests-rtc.c 23) exp 360 was 0
...................................................................................................................................................................................................................................................................................................................................................................................................................................................
run 1037 failures 2
Timeout in expect script at "child.expect(r'OK ((\d+) tests)', timeout=timeout)" (dist/pythonlibs/testrunner/init.py:57)

Stderr

--------------------------------- Captured Err ---------------------------------
make: *** [/home/runner/work/RIOT/RIOT/RIOT/makefiles/tests/tests.inc.mk:22: test] Error 1

Test case:	test_task03[nodes43-tests-rtc]
Outcome:	Failed
Duration:	128.0 sec
Failed	subprocess.CalledProcessError: Command '['make', '--no-print-directory', '-C', '/home/runner/work/RIOT/RIOT/RIOT/tests/unittests', 'tests-rtc', 'flash-only', 'test']' returned non-zero exit status 2.

nodes = [<riotctrl.ctrl.RIOTCtrl object at 0x7faa55d12250>], log_nodes = True
riotbase = '/home/runner/work/RIOT/RIOT/RIOT', test_suite = 'tests-rtc'
@pytest.mark.parametrize('nodes, test_suite',
                         [pytest.param(['native'], t) for t in TESTS_SUITES],
                         indirect=['nodes'])
def test_task03(nodes, log_nodes, riotbase, test_suite):

>       run_unittests(test_suite, nodes, log_nodes, os.path.join(riotbase, APP))
01-ci/test_spec01.py:39:

01-ci/test_spec01.py:19: in run_unittests

node.make_run(

.tox/test/lib/python3.9/site-packages/riotctrl/ctrl.py:178: in make_run

return subprocess.run(command, env=self.env, *runargs, **runkwargs)

input = None, capture_output = False, timeout = None, check = True

popenargs = (['make', '--no-print-directory', '-C', '/home/runner/work/RIOT/RIOT/RIOT/tests/unittests', 'tests-rtc', 'flash-only', ...],)

kwargs = {'env': {'BOARD': 'native', 'BUILD_IN_DOCKER': '1', 'DOCKER_ENV_VARS': 'USEMODULE', 'DOCKER_MAKE_ARGS': '-j', ...}, 'stderr': None, 'stdout': None}

process = <Popen: returncode: 2 args: ['make', '--no-print-directory', '-C', '/home/ru...>

stdout = None, stderr = None, retcode = 2
def run(*popenargs,
        input=None, capture_output=False, timeout=None, check=False, **kwargs):
    """Run command with arguments and return a CompletedProcess instance.

    The returned instance will have attributes args, returncode, stdout and
    stderr. By default, stdout and stderr are not captured, and those attributes
    will be None. Pass stdout=PIPE and/or stderr=PIPE in order to capture them.

    If check is True and the exit code was non-zero, it raises a
    CalledProcessError. The CalledProcessError object will have the return code
    in the returncode attribute, and output &amp; stderr attributes if those streams
    were captured.

    If timeout is given, and the process takes too long, a TimeoutExpired
    exception will be raised.

    There is an optional argument "input", allowing you to
    pass bytes or a string to the subprocess's stdin.  If you use this argument
    you may not also use the Popen constructor's "stdin" argument, as
    it will be used internally.

    By default, all communication is in bytes, and therefore any "input" should
    be bytes, and the stdout and stderr will be bytes. If in text mode, any
    "input" should be a string, and stdout and stderr will be strings decoded
    according to locale encoding, or by "encoding" if set. Text mode is
    triggered by setting any of text, encoding, errors or universal_newlines.

    The other arguments are the same as for the Popen constructor.
    """
    if input is not None:
        if kwargs.get('stdin') is not None:
            raise ValueError('stdin and input arguments may not both be used.')
        kwargs['stdin'] = PIPE

    if capture_output:
        if kwargs.get('stdout') is not None or kwargs.get('stderr') is not None:
            raise ValueError('stdout and stderr arguments may not be used '
                             'with capture_output.')
        kwargs['stdout'] = PIPE
        kwargs['stderr'] = PIPE

    with Popen(*popenargs, **kwargs) as process:
        try:
            stdout, stderr = process.communicate(input, timeout=timeout)
        except TimeoutExpired as exc:
            process.kill()
            if _mswindows:
                # Windows accumulates the output in a single blocking
                # read() call run on child threads, with the timeout
                # being done in a join() on those threads.  communicate()
                # _after_ kill() is required to collect that and add it
                # to the exception.
                exc.stdout, exc.stderr = process.communicate()
            else:
                # POSIX _communicate already populated the output so
                # far into the TimeoutExpired exception.
                process.wait()
            raise
        except:  # Including KeyboardInterrupt, communicate handled that.
            process.kill()
            # We don't call process.wait() as .__exit__ does that for us.
            raise
        retcode = process.poll()
        if check and retcode:

>               raise CalledProcessError(retcode, process.args,

output=stdout, stderr=stderr)

E               subprocess.CalledProcessError: Command '['make', '--no-print-directory', '-C', '/home/runner/work/RIOT/RIOT/RIOT/tests/unittests', 'tests-rtc', 'flash-only', 'test']' returned non-zero exit status 2.
/opt/hostedtoolcache/Python/3.9.2/x64/lib/python3.9/subprocess.py:528: CalledProcessError

Stdout

--------------------------------- Captured Log ---------------------------------

--------------------------------- Captured Out ---------------------------------
Launching build container using image "riot/riotbuild:latest".
docker run --rm --tty --user $(id -u) -v '/usr/share/zoneinfo/Etc/UTC:/etc/localtime:ro' -v '/home/runner/work/RIOT/RIOT/RIOT:/data/riotbuild/riotbase:delegated' -e 'RIOTBASE=/data/riotbuild/riotbase' -e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' -e 'BUILD_DIR=/data/riotbuild/riotbase/build' -e 'RIOTPROJECT=/data/riotbuild/riotbase' -e 'RIOTCPU=/data/riotbuild/riotbase/cpu' -e 'RIOTBOARD=/data/riotbuild/riotbase/boards' -e 'RIOTMAKE=/data/riotbuild/riotbase/makefiles' -e 'BOARD=native' -w '/data/riotbuild/riotbase/tests/unittests/' 'riot/riotbuild:latest' make -j tests-rtc
#x1B[1;32mBuilding application "tests_unittests" for "native" with MCU "native".#x1B[0m

"make" -C /data/riotbuild/riotbase/boards/native
"make" -C /data/riotbuild/riotbase/core
"make" -C /data/riotbuild/riotbase/cpu/native
"make" -C /data/riotbuild/riotbase/drivers
"make" -C /data/riotbuild/riotbase/sys
"make" -C /data/riotbuild/riotbase/tests/unittests/tests-rtc
"make" -C /data/riotbuild/riotbase/cpu/native/periph
"make" -C /data/riotbuild/riotbase/cpu/native/stdio_native
"make" -C /data/riotbuild/riotbase/boards/native/drivers
"make" -C /data/riotbuild/riotbase/sys/embunit
"make" -C /data/riotbuild/riotbase/sys/test_utils/interactive_sync
"make" -C /data/riotbuild/riotbase/drivers/periph_common
text data bss dec hex filename
36180 768 51868 88816 15af0 /data/riotbuild/riotbase/tests/unittests/bin/native/tests_unittests.elf
r
/home/runner/work/RIOT/RIOT/RIOT/tests/unittests/bin/native/tests_unittests.elf /dev/ttyACM0
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: cb25-HEAD)
Help: Press s to start test, r to print it is ready
READY
s
START
.
rtc_tests.test_rtc_compat (tests/unittests/tests-rtc/tests-rtc.c 29) exp 0 was 6
.
rtc_tests.test_rtc_sec_wrap (tests/unittests/tests-rtc/tests-rtc.c 23) exp 360 was 0
.......
run 9 failures 2
Timeout in expect script at "child.expect(r'OK ((\d+) tests)', timeout=timeout)" (dist/pythonlibs/testrunner/init.py:57)

Stderr

--------------------------------- Captured Err ---------------------------------
make: *** [/home/runner/work/RIOT/RIOT/RIOT/makefiles/tests/tests.inc.mk:22: test] Error 1

miri64 · 2021-03-10T10:37:31Z

mega-xplained needs to be added to tests/pkg_microcoap's Makefile.ci.

miri64 · 2021-03-10T10:40:11Z

Until then, I also started a run on the testbed

The fails there also seem unrelated and fail as far as I can tell also in the latest weekly.

MrKevinWeiss · 2021-06-24T11:46:03Z

This would be nice to have in the release

fjmolinas · 2021-06-28T06:53:36Z

@kaspar030 this needs a rebase!

OlegHahm · 2022-03-09T12:56:20Z

OMG! I've been detached for too long. What happened to our beloved "no bloody macros, please!"-policy?

kaspar030 · 2022-03-09T13:24:15Z

OMG! I've been detached for too long. What happened to our beloved "no bloody macros, please!"-policy?

Fighting hard for it on every occasion. But, putting information in "global space" is difficult. We've been looking for a solution for years.
My initial try of doing it via compile time code generation (remember .c.snip?) didn't fly. So, the linker is now used. And it needs macros for convenience, doing this by hand is a bit unwieldy.

The upsides are pretty great - no more auto_init style ifdef messes, code like shell commands can stay with their modules, can be added without changing "shell" itself, or outside the tree. IMO, worth the one-line macro invocations.

I do prefer type safe alternatives, but there's nothing like that for plain C.

fjmolinas · 2022-04-07T13:37:09Z

Needs a rebase now, besides tests was this waiting for something else?

kaspar030 · 2022-06-08T08:32:38Z

done in #18152.

kaspar030 requested review from aabadie, fjmolinas, leandrolanzieri, MichelRottleuthner, miri64, MrKevinWeiss and smlng as code owners February 25, 2021 12:05

kaspar030 force-pushed the convert_shell_commands_to_xfa branch from 185b452 to 69651d2 Compare February 25, 2021 13:51

miri64 reviewed Mar 2, 2021

View reviewed changes

sys/shell/commands/shell_commands.c Show resolved Hide resolved

sys/include/shell.h Outdated Show resolved Hide resolved

sys/shell/commands/shell_commands.c Show resolved Hide resolved

kaspar030 force-pushed the convert_shell_commands_to_xfa branch from f29f73f to 20c0427 Compare March 2, 2021 10:46

miri64 mentioned this pull request Mar 3, 2021

shell: provide submodule for each shell command #16135

Closed

kaspar030 added 5 commits March 9, 2021 14:58

sys/shell: convert in_tree shell commands to XFA

014d357

Revert "tests/periph_spi_dma: blacklist samd10-xmini"

50de138

This reverts commit d9cda53.

tests/ps_schedstatistics: fix command order in shell help test

82b2c16

tests/shell: fix expected command order in test

6bd38b1

tests/pkg_microcoap: memory blacklist atmega1284p

cb25c8d

kaspar030 force-pushed the convert_shell_commands_to_xfa branch from b866b7a to cb25c8d Compare March 9, 2021 13:59

kaspar030 removed the State: waiting for other PR State: The PR requires another PR to be merged first label Mar 9, 2021

tests/pkg_microcoap: memory-blacklist mega-xplained

9d6738c

MrKevinWeiss added this to the Release 2021.07 milestone Jun 21, 2021

MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021

fjmolinas added this to the Release 2022.01 milestone Nov 18, 2021

emmanuelsearch modified the milestones: Release 2022.01, Release 2022.04 Jan 27, 2022

maribu mentioned this pull request May 31, 2022

sys/shell_commands: convert to SHELL_COMMAND() #18152

Merged

kaspar030 closed this Jun 8, 2022

kaspar030 deleted the convert_shell_commands_to_xfa branch June 8, 2022 08:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sys/shell: convert shell_commands.c commands to use XFA #16095

sys/shell: convert shell_commands.c commands to use XFA #16095

kaspar030 commented Feb 25, 2021 •

edited

Loading

kaspar030 commented Feb 25, 2021 •

edited

Loading

miri64 commented Feb 25, 2021

kaspar030 commented Mar 2, 2021

miri64 commented Mar 2, 2021

miri64 commented Mar 2, 2021

miri64 commented Mar 2, 2021

kaspar030 commented Mar 2, 2021

miri64 commented Mar 2, 2021

miri64 commented Mar 2, 2021

kaspar030 commented Mar 2, 2021

kaspar030 commented Mar 2, 2021

kaspar030 commented Mar 2, 2021

miri64 commented Mar 2, 2021

kaspar030 commented Mar 2, 2021

miri64 commented Mar 2, 2021

miri64 commented Mar 9, 2021

kaspar030 commented Mar 9, 2021

miri64 commented Mar 9, 2021

miri64 commented Mar 9, 2021

miri64 commented Mar 10, 2021

01-ci.test_spec01

miri64 commented Mar 10, 2021

miri64 commented Mar 10, 2021

MrKevinWeiss commented Jun 24, 2021

fjmolinas commented Jun 28, 2021

OlegHahm commented Mar 9, 2022

kaspar030 commented Mar 9, 2022

fjmolinas commented Apr 7, 2022

kaspar030 commented Jun 8, 2022

sys/shell: convert shell_commands.c commands to use XFA #16095

sys/shell: convert shell_commands.c commands to use XFA #16095

Conversation

kaspar030 commented Feb 25, 2021 • edited Loading

Contribution description

Testing procedure

Issues/PRs references

kaspar030 commented Feb 25, 2021 • edited Loading

miri64 commented Feb 25, 2021

kaspar030 commented Mar 2, 2021

miri64 commented Mar 2, 2021

miri64 commented Mar 2, 2021

miri64 commented Mar 2, 2021

kaspar030 commented Mar 2, 2021

miri64 commented Mar 2, 2021

miri64 commented Mar 2, 2021

kaspar030 commented Mar 2, 2021

kaspar030 commented Mar 2, 2021

kaspar030 commented Mar 2, 2021

miri64 commented Mar 2, 2021

kaspar030 commented Mar 2, 2021

miri64 commented Mar 2, 2021

miri64 commented Mar 9, 2021

kaspar030 commented Mar 9, 2021

miri64 commented Mar 9, 2021

miri64 commented Mar 9, 2021

miri64 commented Mar 10, 2021

01-ci.test_spec01

miri64 commented Mar 10, 2021

miri64 commented Mar 10, 2021

MrKevinWeiss commented Jun 24, 2021

fjmolinas commented Jun 28, 2021

OlegHahm commented Mar 9, 2022

kaspar030 commented Mar 9, 2022

fjmolinas commented Apr 7, 2022

kaspar030 commented Jun 8, 2022

kaspar030 commented Feb 25, 2021 •

edited

Loading

kaspar030 commented Feb 25, 2021 •

edited

Loading