shell_commands: include RSSI in ping output

[benpicco]

I was looking for a quick way to gauge the link quality between two nodes, so I added the rssi to the output of the ping6 command.

Maybe this is generally useful to have.

2019-03-04 11:29:53,484 - INFO #  ping6 fe80::7841:4946:59b8:3d9c
2019-03-04 11:29:53,498 - INFO # 12 bytes from fe80::7841:4946:59b8:3d9c: icmp_seq=0 ttl=64 rssi=-51 dBm time=5.975 ms
2019-03-04 11:29:54,498 - INFO # 12 bytes from fe80::7841:4946:59b8:3d9c: icmp_seq=1 ttl=64 rssi=-51 dBm time=5.631 ms
2019-03-04 11:29:55,500 - INFO # 12 bytes from fe80::7841:4946:59b8:3d9c: icmp_seq=2 ttl=64 rssi=-49 dBm time=7.273 ms
2019-03-04 11:29:55,500 - INFO # 
2019-03-04 11:29:55,504 - INFO # --- fe80::7841:4946:59b8:3d9c PING statistics ---
2019-03-04 11:29:55,509 - INFO # 3 packets transmitted, 3 packets received, 0% packet loss
2019-03-04 11:29:55,515 - INFO # round-trip min/avg/max = 5.631/6.293/7.273 ms

[maribu]

To me the RSSI info would be a nice addition to the ping info :-)
Please have a look at the inline comments.

Changes addressed :-)

[maribu]

@benpicco: One remark for future PRs: Especially in bigger PRs it is helpful for the review to address the changes in separate commits and then squash one the reviewer acknowleged the changes were addressed. The work flow would be like this:

Lets assume you have commits 4242 and 1337 in your PR. First you would apply changes related to 4242, then you would do

git commit --fixup=4242

and similar the changes related to 1337 would be commited by git commit --fixup=1337. Once the reveiwer asks for squashing, you can do so by running git rebase -i --autosquash <ID>, where <ID> is the commit before your first commit, e.g. here one commit before 4242. This would integrated the fixups into the corresponding commits. (Of course, this would change the hash so a force push will be required.)

[maribu]

OK, it does not work for the MSB-A2 using "my" cc110x driver. The driver passes -35 dBm, but ping reports 2 dBm. I'll investigate later. Could be just as well my fault. (Also: The pings might end up being fragmented when 6LoWPAN is used. I wonder what the RSSI value will be in that case.)

[maribu]

@miri64: Please also have a look at this. The console output of ping6 is now fully identical to the output of the Linux ping6 (or at least for the busybox version of it). This PR would change this. I personally see no harm in that, as IMHO the output of ping6 is intended to be "parsed" by humans. Others might disagree on that and will end up with broken scripts.

(To be honest: When properly parsing the output of ping6 as a space separated list of key=value pairs, this should not break their scripts. So one could argue this would only break buggy scripts.)

[miri64]

I'm fine with this. Most parsing scripts I'm aware of just parse the statistics in the end anyway.

[miri64]

iputils's ping6 itself also can have different per-packet-output (e.g. with -f)

[SemjonWilke]

If I checkout and try this on nrf52840dk i get a FAILED ASSERTION on

RIOT/sys/net/gnrc/routing/rpl/gnrc_rpl_auto_init.c

Line 32 in f7cdbda

assert(netif != NULL);

Probably rebase?

[benpicco]

auto_init_gnrc_rpl() should run before any shell command is run and this doesn't touch anything outside sc_gnrc_icmpv6_echo.c, so I think it's unrelated.

[maribu]

I think @SemjonKerner is pointing out that this PR was created at a point in time the nrf driver was not yet included in RIOT's master. Rebasing this PR on top of the current master would allow using it.

I personally have no strong opinion about rebasing. The pro would be that testers would not need to rebase locally and the testing results are theoretically more trustful if the exact state of the PR is tested. The con is that force pushs during PRs would require the reviewers to look at the source code again to rule out some mistakes sneaked in - (this happens more frequently than one would guess and also happened to me).

Btw: Sorry for stalling with the cc110x test. As @bergzand referenced this PR to test the RSSI output of nrf802154, I guess the issue with the cc110x is unrelated to this PR. Maybe @bergzand can tick the testing label if I'm this did indeed work with another transceiver?

[SemjonWilke]

auto_init_gnrc_rpl() should run before any shell command is run and this doesn't touch anything outside sc_gnrc_icmpv6_echo.c, so I think it's unrelated.

Uhm, thats not really the problem. The nrf52840dk just got a 802.15.4 radio driver and your branch is not up to date with that.
When I rebase your branch onto master everything works as expected and RSSI is printed correctly.

[maribu]

When I rebase your branch onto master everything works as expected and RSSI is printed correctly.

@SemjonKerner: Thanks for testing!

[maribu]

ACK. Once Murdock gives green light I'll hit merge