added internet access detection before version checking is performed #199

nitin710 · 2023-04-20T15:27:03Z

Description

Fixes a bug where the Oscilloscope hangs while trying to check for the latest available version when there is no internet access.
Different host+router combinations show different behavior.
Some Hosts show an "infinite wait" while trying to detect the software version.
With this patch, we first detect internet access within a timeout period. Version check is performed ONLY if internet access is detected.
Removes creation of txt file with software version.

Requirements

None

Issues Referenced

Fixes Spurious behavior for software version check on networks without internet access #194

Documentation update

None

Notes for Reviewer

successful ping outputs: added internet access detection before version checking is performed #199 (comment)
failed ping outputs: added internet access detection before version checking is performed #199 (comment)

Algorithm

the IP address we ping is 8.8.8.8. It is google's public DNS. We are using ping with a specific IP (and not a domain name) as that removes undefined wait periods associated with DNS resolution. More details on google's DNS : https://developers.google.com/speed/public-dns
we look for the keywords TTL/ttl in the ping response since only successful responses have those keywords
Concerns
- The target string, while present in current ping responses may possibly be changed in a future version of ping. It would mean that when it happens, the software version checker will stop working. Consequently, EmotiBit Oscilloscope will work, just not check for new version updates.

Testing

Results

#199 (comment)

OS	sw version	Network with internet	network without internet	no network
Expectation	-	(new version available notification + opens link to latest release)	skips version check	skips version check
win	1.8.1.fix-swVersionChecker.2	✔️	✔️	✔️
osx	1.8.1.fix-swVersionChecker.2	✅	✅	✅
linux	1.8.1.fix-swVersionChecker.2	✅	✅	✅

Unit Tests

Unit tests performed (from EmotiBit Feature Test Protocols)

✔️ Version checker with network without internet access
✔️ Version checker without network connection
✔️ Version checker with network+internet access

The corresponding results are recorded in the EmotiBit Software Testing Records sheet.

Steps to test

Test the behavior of the EmotiBit Oscilloscope when the host is connected to a network according to the criteria listed above.

Shared files

None

Checklist to allow merge

All dependent repositories used were on branch master
Software
- Get approval from the reviewer
- Passed testing on Windows
- Passed testing on macOS (for major changes/GUI changes/ PRs adding files distributed with the EmotiBit software)
- Passed testing on linux (ubuntu) (for major changes/GUI changes/ PRs adding files distributed with the EmotiBit software)
- Update software bundle version in ofxEmotiBitVersion.h
doxygen style comments included for new code snippets
Required documentation updated

Screenshots:

EmotiBitOscilloscope/src/ofApp.cpp

pulling latest master

produceconsumerobot · 2023-06-09T21:39:20Z

Review 01

@nitin710 what OSes did you test this on?
On Mac it seems like there may be a different message?

@nitin710 I'm getting build errors that reference missing ofxLSL.cpp

Is it possible that you forgot to check-in the updated project file?

nitin710 · 2023-06-15T12:28:38Z

I need to test this with macOS (and linux for that matter). I did not originally test it with macOS.
RE "I'm getting build errors that reference missing ofxLSL.cpp"
- There are no changes dependent on ofxLSL. Can you try a clean build?

EmotiBitOscilloscope/src/ofApp.cpp

nitin710 · 2023-11-29T20:02:35Z

Successful ping outputs

linux

ping -c 3 8.8.8.8
PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data.
64 bytes from 8.8.8.8: icmp_seq=1 ttl=54 time=18.2 ms
64 bytes from 8.8.8.8: icmp_seq=2 ttl=54 time=25.7 ms
64 bytes from 8.8.8.8: icmp_seq=3 ttl=54 time=14.5 ms

--- 8.8.8.8 ping statistics ---
3 packets transmitted, 3 received, 0% packet loss, time 2003ms
rtt min/avg/max/mdev = 14.503/19.471/25.685/4.649 ms

macOS ventura M1

ping -c 3 8.8.8.8
PING 8.8.8.8 (8.8.8.8): 56 data bytes
64 bytes from 8.8.8.8: icmp_seq=0 ttl=54 time=30.321 ms
64 bytes from 8.8.8.8: icmp_seq=1 ttl=54 time=25.459 ms
64 bytes from 8.8.8.8: icmp_seq=2 ttl=54 time=21.433 ms

--- 8.8.8.8 ping statistics ---
3 packets transmitted, 3 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 21.433/25.738/30.321/3.634 ms

macOS big sur (intel)

ping -c 3 8.8.8.8
PING 8.8.8.8 (8.8.8.8): 56 data bytes
64 bytes from 8.8.8.8: icmp_seq=0 ttl=54 time=21.199 ms
64 bytes from 8.8.8.8: icmp_seq=1 ttl=54 time=19.552 ms
64 bytes from 8.8.8.8: icmp_seq=2 ttl=54 time=25.092 ms

--- 8.8.8.8 ping statistics ---
3 packets transmitted, 3 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 19.552/21.948/25.092/2.323 ms

macOS mojave (intel)

ping -c 3 8.8.8.8
PING 8.8.8.8 (8.8.8.8): 56 data bytes
64 bytes from 8.8.8.8: icmp_seq=0 ttl=54 time=21.740 ms
64 bytes from 8.8.8.8: icmp_seq=1 ttl=54 time=11.917 ms
64 bytes from 8.8.8.8: icmp_seq=2 ttl=54 time=20.419 ms

--- 8.8.8.8 ping statistics ---
3 packets transmitted, 3 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 11.917/18.025/21.740/4.353 ms

macOS monterey (intel)

ping -c 3 8.8.8.8
PING 8.8.8.8 (8.8.8.8): 56 data bytes
64 bytes from 8.8.8.8: icmp_seq=0 ttl=54 time=24.305 ms
64 bytes from 8.8.8.8: icmp_seq=1 ttl=54 time=14.307 ms
64 bytes from 8.8.8.8: icmp_seq=2 ttl=54 time=15.809 ms

--- 8.8.8.8 ping statistics ---
3 packets transmitted, 3 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 14.307/18.140/24.305/4.402 ms

Windows

ping -n 3 8.8.8.8
Pinging 8.8.8.8 with 32 bytes of data:
Reply from 8.8.8.8: bytes=32 time=20ms TTL=54
Reply from 8.8.8.8: bytes=32 time=17ms TTL=54
Reply from 8.8.8.8: bytes=32 time=15ms TTL=54

Ping statistics for 8.8.8.8:
    Packets: Sent = 3, Received = 3, Lost = 0 (0% loss),
Approximate round trip times in milli-seconds:
    Minimum = 15ms, Maximum = 20ms, Average = 17ms```

nitin710 · 2023-11-30T17:48:38Z

Failed ping outputs

Linux (WiFi OFF)

ping: connect: Network is unreachable

macOS Ventura (Apple silicon) (WiFi OFF)

PING 8.8.8.8 (8.8.8.8): 56 data bytes
ping: sendto: No route to host

--- 8.8.8.8 ping statistics ---
1 packets transmitted, 0 packets received, 100.0% packet loss

macOS mojave (intel) (WiFi OFF)

PING 8.8.8.8 (8.8.8.8): 56 data bytes
ping: sendto: No route to host

--- 8.8.8.8 ping statistics ---
1 packets transmitted, 0 packets received, 100.0% packet loss

Windows (WiFi OFF)


C:\Users\nitin>ping -n 1 8.8.8.8

Pinging 8.8.8.8 with 32 bytes of data:
Reply from 192.168.1.197: Destination host unreachable.

Ping statistics for 8.8.8.8:
    Packets: Sent = 1, Received = 1, Lost = 0 (0% loss), // NOTE THAT WE SOMEHOW got a packet back, BUT there was no internet access

C:\Users\nitin>ping -n 1 8.8.8.8

Pinging 8.8.8.8 with 32 bytes of data:
Reply from 192.168.1.197: Destination host unreachable.

Ping statistics for 8.8.8.8:
    Packets: Sent = 1, Received = 1, Lost = 0 (0% loss),  // REPEATED experiment with same result

C:\Users\nitin>ping -n 2 8.8.8.8

Pinging 8.8.8.8 with 32 bytes of data:
Request timed out.
Reply from 192.168.1.197: Destination host unreachable. 

Ping statistics for 8.8.8.8:
    Packets: Sent = 2, Received = 1, Lost = 1 (50% loss), // sending more than 1 packet results in failure of 1

C:\Users\nitin>ping -w 2000 -n 3 8.8.8.8

Pinging 8.8.8.8 with 32 bytes of data:
Request timed out.
Request timed out.
Request timed out.

Ping statistics for 8.8.8.8:
    Packets: Sent = 3, Received = 0, Lost = 3 (100% loss), // In this case, all 3 were lost. Hypothesis: It can be that all packets may be received erroneously? This will put us in the bucket of "internet connection exists", when there isn't any.

C:\Users\nitin>ping -n 3 8.8.8.8

Pinging 8.8.8.8 with 32 bytes of data:
Request timed out.
Reply from 192.168.1.197: Destination host unreachable.
Request timed out.

Ping statistics for 8.8.8.8:
    Packets: Sent = 3, Received = 1, Lost = 2 (66% loss), // Another random loss %

C:\Users\nitin>ping -n 1 8.8.8.8

Pinging 8.8.8.8 with 32 bytes of data:
Reply from 192.168.1.197: Destination host unreachable.

Ping statistics for 8.8.8.8:
    Packets: Sent = 1, Received = 1, Lost = 0 (0% loss),  // ??


C:\Users\nitin>ping -w 100 -n 3 8.8.8.8

Pinging 8.8.8.8 with 32 bytes of data:
Request timed out.
Request timed out.
Request timed out.

Ping statistics for 8.8.8.8:
    Packets: Sent = 3, Received = 0, Lost = 3 (100% loss),

C:\Users\nitin>ping -w 100 -n 1 8.8.8.8

Pinging 8.8.8.8 with 32 bytes of data:
Reply from 192.168.1.197: Destination host unreachable.

Ping statistics for 8.8.8.8:
    Packets: Sent = 1, Received = 1, Lost = 0 (0% loss), // still reachable if pinging only once??


================NETWORK CONNECTED=============
C:\Users\nitin>ping -w 100 -n 1 8.8.8.8  

Pinging 8.8.8.8 with 32 bytes of data:
Reply from 8.8.8.8: bytes=32 time=28ms TTL=54

Ping statistics for 8.8.8.8:
    Packets: Sent = 1, Received = 1, Lost = 0 (0% loss),
Approximate round trip times in milli-seconds:     // This statement is only printed when the network exists
    Minimum = 28ms, Maximum = 28ms, Average = 28ms

Summary

linux/macos seem to correctly reflect the absence of network from the ping statistics.
Windows is giving random false positives on packets received.
- It may be possible that it receives all packets sent even when there is no network
- Therefore, we may not be able to use ping statistics to detect network

nitin710 · 2023-12-04T20:53:25Z

Tested with

std::string testString =
		"PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data.\n"
		"64 bytes from 8.8.8.8: icmp_seq = 1 ttl = 54 time = 18.2 ms\n"
		"64 bytes from 8.8.8.8 : icmp_seq = 2 ttl = 54 time = 25.7 ms\n"
		"64 bytes from 8.8.8.8 : icmp_seq = 3 ttl = 54 time = 14.5 ms\n"
		"-- - 8.8.8.8 ping statistics-- -\n"
		"3 packets transmitted, 3 received, 0 % packet loss, time 2003ms\n"
		"rtt min / avg / max / mdev = 14.503 / 19.471 / 25.685 / 4.649 ms";

	/*testString =
		"Pinging 8.8.8.8 with 32 bytes of data:\n"
		"Reply from 8.8.8.8: bytes = 32 time = 20ms TTL = 54\n"
		"Reply from 8.8.8.8 : bytes = 32 time = 17ms TTL = 54\n"
		"Reply from 8.8.8.8 : bytes = 32 time = 15ms TTL = 54\n"
		"Ping statistics for 8.8.8.8 :\n"
		"Packets : Sent = 3, Received = 3, Lost = 0 (0 % loss),\n"
		"Approximate round trip times in milli - seconds :\n"
		"Minimum = 15ms, Maximum = 20ms, Average = 17ms";*/

	/*testString =
		"PING 8.8.8.8 (8.8.8.8) : 56 data bytes\n"
		"ping : sendto: No route to host\n"

		"-- - 8.8.8.8 ping statistics-- -\n"
		"1 packets transmitted, 0 packets received, 100.0% packet loss";*/

	// Linux
	/*testString =
		"ping : connect: Network is unreachable";*/

	// Windows
	testString =
		"Pinging 8.8.8.8 with 32 bytes of data :\n"
		"Request timed out.\n"
		"Request timed out.\n"
		"Request timed out.\n"

		"Ping statistics for 8.8.8.8 :\n"
		"Packets : Sent = 3, Received = 0, Lost = 3 (100 % loss),\n";

…to compile softwareVersionChecker file

nitin710 · 2023-12-08T18:29:34Z

Testing Matrix

OS	sw version	Network with internet	network without internet	no network
Expectation	-	(new version available notification + opens link to latest release)	skips version check	skips version check
win	1.8.1.fix-swVersionChecker.2	✔️	✔️	✔️
osx	1.8.1.fix-swVersionChecker.2	✅	✅	✅
linux	1.8.1.fix-swVersionChecker.2	✅	✅	✅

nitin710 · 2023-12-08T18:54:53Z

EmotiBitOscilloscope/src/ofApp.cpp

-		ofLogError("An exception occured while checking latest version of EmotiBit software");
-	}
-}
+


This function has been moved to its own file in ofxEmotiBit/src.
Idea being, we can re-use it in other softwares (parser)

nitin710 · 2023-12-08T18:55:48Z

src/SoftwareVersionChecker.cpp

+void SoftwareVersionChecker::checkLatestVersion()
+{
+	bool isNetworkAvailable = false;
+	// -n specifies number of tries. we are going to try to detect internet connection by sending 1 packet


For reviewer:
This section is just ported over from ofApp.
See SoftwareVersionChecker::testPingResponse(std::string pingResponse) for new code

nitin710 · 2024-02-07T22:03:24Z

src/SoftwareVersionChecker.cpp

+	*/
+	bool isNetworkAvailable = false;
+
+	if (pingResponse.find("time") != std::string::npos && (pingResponse.find("TTL") != std::string::npos || pingResponse.find("ttl") != std::string::npos))


Core algorithm for detecting internet connection.

produceconsumerobot · 2024-02-27T00:24:40Z

Review 02

@nitin710 review complete

Why are we looking for "time"? Looking at the examples, it seems like TTL/ttl is sufficient to separate success vs failed ping, no?

nitin710 · 2024-03-05T19:37:57Z

Why are we looking for "time"? Looking at the examples, it seems like TTL/ttl is sufficient to separate success vs failed ping, no?

I could not find and dev comments explaining the reasoning. I think it was an attempt at "stricter check", but that is clearly not the case.
Removed it from the checking.

nitin710 · 2024-03-28T01:41:34Z

This PR is already merged into dev.
Will be merged to master once build on linux is resolved.

nitin710 added 4 commits April 20, 2023 09:24

added inaternet access detection before version checking is performed

4b74871

comment update

dd17f0a

removed creation of version file

9c115c9

minor typo in console output fixed.

a88f19e

nitin710 commented Apr 20, 2023

View reviewed changes

EmotiBitOscilloscope/src/ofApp.cpp Outdated Show resolved Hide resolved

nitin710 commented Apr 20, 2023

View reviewed changes

EmotiBitOscilloscope/src/ofApp.cpp Outdated Show resolved Hide resolved

nitin710 assigned produceconsumerobot Apr 20, 2023

Merge branch 'master' into fix-swVersionChecker

4aa928d

pulling latest master

nitin710 changed the title ~~added inaternet access detection before version checking is performed~~ added internet access detection before version checking is performed Apr 21, 2023

nitin710 commented Nov 28, 2023

View reviewed changes

EmotiBitOscilloscope/src/ofApp.cpp Outdated Show resolved Hide resolved

testPing response based on sent and received packets

444f5b2

This comment was marked as outdated.

Sign in to view

nitin710 added 2 commits December 4, 2023 15:10

moved version checker code to separate file

08864f6

compiling version checker

21eabb8

nitin710 added 4 commits December 4, 2023 18:07

version checker moved to separate file. tested working

1c59413

vs project updated to registed new file

1f3f5f3

fixed ping command options for unix systems. updated xcode proj file …

0cfa312

…to compile softwareVersionChecker file

updated ping command for linux

0e0b231

nitin710 commented Dec 8, 2023

View reviewed changes

nitin710 requested a review from produceconsumerobot December 8, 2023 18:56

comments updated

41ffb2c

nitin710 commented Feb 7, 2024

View reviewed changes

removed checking for "time" form ping response based on review comment

4d26c59

nitin710 mentioned this pull request May 31, 2024

Dev featuring v1.11.1 #239

Merged

nitin710 merged commit b00a16d into master May 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added internet access detection before version checking is performed #199

added internet access detection before version checking is performed #199

nitin710 commented Apr 20, 2023 •

edited

Loading

produceconsumerobot commented Jun 9, 2023 •

edited

Loading

nitin710 commented Jun 15, 2023

nitin710 commented Nov 29, 2023 •

edited

Loading

nitin710 commented Nov 30, 2023 •

edited

Loading

This comment was marked as outdated.

nitin710 commented Dec 4, 2023

nitin710 commented Dec 8, 2023 •

edited

Loading

nitin710 Dec 8, 2023

nitin710 Dec 8, 2023 •

edited

Loading

nitin710 Feb 7, 2024 •

edited

Loading

produceconsumerobot commented Feb 27, 2024 •

edited

Loading

nitin710 commented Mar 5, 2024 •

edited

Loading

nitin710 commented Mar 28, 2024

added internet access detection before version checking is performed #199

added internet access detection before version checking is performed #199

Conversation

nitin710 commented Apr 20, 2023 • edited Loading

Description

Requirements

Issues Referenced

Documentation update

Notes for Reviewer

Algorithm

Testing

Results

Unit Tests

Steps to test

Shared files

Checklist to allow merge

Screenshots:

produceconsumerobot commented Jun 9, 2023 • edited Loading

Review 01

nitin710 commented Jun 15, 2023

nitin710 commented Nov 29, 2023 • edited Loading

Successful ping outputs

linux

macOS ventura M1

macOS big sur (intel)

macOS mojave (intel)

macOS monterey (intel)

Windows

nitin710 commented Nov 30, 2023 • edited Loading

Failed ping outputs

Linux (WiFi OFF)

macOS Ventura (Apple silicon) (WiFi OFF)

macOS mojave (intel) (WiFi OFF)

Windows (WiFi OFF)

Summary

This comment was marked as outdated.

nitin710 commented Dec 4, 2023

nitin710 commented Dec 8, 2023 • edited Loading

Testing Matrix

nitin710 Dec 8, 2023

Choose a reason for hiding this comment

nitin710 Dec 8, 2023 • edited Loading

Choose a reason for hiding this comment

nitin710 Feb 7, 2024 • edited Loading

Choose a reason for hiding this comment

produceconsumerobot commented Feb 27, 2024 • edited Loading

Review 02

nitin710 commented Mar 5, 2024 • edited Loading

nitin710 commented Mar 28, 2024

nitin710 commented Apr 20, 2023 •

edited

Loading

produceconsumerobot commented Jun 9, 2023 •

edited

Loading

nitin710 commented Nov 29, 2023 •

edited

Loading

nitin710 commented Nov 30, 2023 •

edited

Loading

nitin710 commented Dec 8, 2023 •

edited

Loading

nitin710 Dec 8, 2023 •

edited

Loading

nitin710 Feb 7, 2024 •

edited

Loading

produceconsumerobot commented Feb 27, 2024 •

edited

Loading

nitin710 commented Mar 5, 2024 •

edited

Loading