Fix RTC.sh failing ping due to syntax

MiSTer-devel / Scripts_MiSTer

Miscellaneous Bash scripts for MiSTer

GNU General Public License v3.0

85 stars 43 forks source link

Fix RTC.sh failing ping due to syntax #90

Closed birdybro closed 2 years ago

birdybro commented 2 years ago

Current syntax won't work because it can't ping that kind of address, it needs an IP. I also increased the wait time to reduce unnecessary failures upon people taking more than one second to ping to ntp. EDIT: Switched to forcing IPv4 use for ping to solve the issue instead.

sorgelig commented 2 years ago

why it can't ping by name?

birdybro commented 2 years ago

It can, but the error handling of the ping makes it fail due to the error message:

/root# ping -q -w1 -c1 "0.pool.ntp.org"
ping: socket: Address family not supported by protocol
PING 0.pool.ntp.org (129.250.35.251) 56(84) bytes of data.

--- 0.pool.ntp.org ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 50.783/50.783/50.783/0.000 ms

This makes the script fail to proceed with a No Internet Connection and then exit.

I suppose we could adjust the error handling instead, my fix was probably too hasty.

sorgelig commented 2 years ago

yeah, i think it's better to use name instead of IP which may be changed any time.

birdybro commented 2 years ago

Okay, I'll adjust it a bit later, heading off to work in a sec. :)

Thanks!

birdybro commented 2 years ago

https://github.com/iputils/iputils/issues/293 - This is the source of the problem, maybe it would be better to probably find a way to bump iputils version instead? I'm not sure.

If the user has IPv6 disabled then it shouldn't present them with a socket error for IPv6. My workaround worked because it was pointing explicitly to an IPv4 address. MiSTer uses ping 20210202, switching to 20211215 might resolve the issue. From 20211215 release notes:

Alternately, the change I just pushed would seem to resolve it as well.