Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(app): Enable support for IPv4 wired robots by default #2090

Merged
merged 3 commits into from
Aug 22, 2018
Merged

Conversation

mcous
Copy link
Contributor

@mcous mcous commented Aug 20, 2018

overview

This PR enables "new" discovery to support the container changes in #1970 by default in the app. It also adds the changes made in #2072 to the top-level Makefile so that make push-api and make term from the repo-level will automatically work with whatever ethernet robot you have plugged in (whether its a new IPv4 or an old IPv6 one).

Presumably (based on extensive in-house testing and positive results with users who were experiencing issues) fixes #990 and fixes #1964. This is the last known app-side blocker for the 3.3.0 release.

changelog

  • feat(app): Enable support for IPv4 wired robots by default

review requests

The components of this PR have been tested pretty extensively over the past few weeks, but we should be sanity checking this. Testing goal is to get something to break. Please go nuts; we need this to be solid.

If you do test, please leave notes in your comment so we can trace back any checkbox changes.

OSes to test:

  • macOS
  • Ubuntu (or any Ubuntu/Debian based Linux)
    • 14.04 LTS
    • 16.04 LTS
    • 18.04 LTS
  • Windows
    • Windows 10
    • Windows 7

API software versions to test:

  • Tag v3.2.0
  • Current edge

Robot configurations to check out:

  • Ethernet connected legacy IPv6 robot
  • Ethernet connected to IPv4 robot
  • WiFi connected legacy IPv6 robots (WiFi is not IPv6, so should be no actual change here)
  • WiFi connected IPv4 robots
  • Virtual smoothie

Success criteria

  • Robots show up with their actual name (e.g. "opentrons-bitter-wood", not "Opentrons USB")
  • HTTP endpoints (e.g. lights) work
  • RPC endpoints (e.g. tip-probe) work
  • If RPC socket is closed by the server, user is alerted that connection has been lost
    • Simulate by killing process on robot
  • If network connectivity goes down without socket closing, user is alerted
    • Simulate by disabling WiFi or pulling the USB cord

@mcous mcous added feature Ticket is a feature request / PR introduces a feature app Affects the `app` project ready for review labels Aug 20, 2018
@codecov
Copy link

codecov bot commented Aug 20, 2018

Codecov Report

Merging #2090 into edge will decrease coverage by 0.37%.
The diff coverage is 63.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge    #2090      +/-   ##
==========================================
- Coverage   32.86%   32.48%   -0.38%     
==========================================
  Files         453      452       -1     
  Lines        7312     7264      -48     
==========================================
- Hits         2403     2360      -43     
+ Misses       4909     4904       -5
Impacted Files Coverage Δ
app/src/robot/actions.js 91.89% <ø> (-0.42%) ⬇️
app/src/pages/Robots/index.js 0% <ø> (ø) ⬆️
app/src/pages/Robots/RobotSettings.js 0% <ø> (ø) ⬆️
app/src/config/index.js 50% <ø> (ø) ⬆️
app/src/robot/api-client/client.js 86.43% <ø> (-0.07%) ⬇️
app-shell/src/config.js 25% <ø> (ø) ⬆️
app-shell/src/discovery.js 89.65% <0%> (-0.35%) ⬇️
app/src/pages/Calibrate/Pipettes.js 0% <0%> (ø) ⬆️
app/src/components/calibrate-pipettes/index.js 0% <0%> (ø) ⬆️
app/src/components/calibrate-pipettes/Pipettes.js 0% <0%> (ø) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7fdce05...9ca01a4. Read the comment docs.

@mcous
Copy link
Contributor Author

mcous commented Aug 21, 2018

Know problem: app crashes if currently connected robot becomes unhealthy (and, thus, disappears from the list). Working on a fix now

@sfoster1
Copy link
Member

Pass on 3.3 OSX with the sole issue of sometimes if the robot disconnects (either by unplugging the USB or by disconnecting my wifi) the robot disconnected modal won't pop up - the connected list clears but there's no modal. Upon reconnecting, I get the modal.

@Laura-Danielle
Copy link
Contributor

Tested on Windows 10.

Connected successfully to legacy IPv6 + robots on IPv4. Something I found while testing:

I could not set advanced settings on the legacy robot (which should have advanced settings API logic according to SW version).

Copy link
Contributor

@btmorr btmorr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌮

  • make term works from OSX over ethernet with Dockerfile 3.3
  • configuring wifi works over wifi and usb
  • Run App connects and can tip probe, upload protocols, run, and cancel run

@IanLondon
Copy link
Contributor

WIP review:

Tested on Ubuntu 16.04 (well, Linux Mint edition)

Only tested IPv4 (Sunset) b/c we're still working on getting an IPv6 set up -- both ethernet & wifi work for all the success criteria listed.

Virtual smoothie also OK for all success criteria.


Also, looks like there's a merge conflict RN

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app Affects the `app` project feature Ticket is a feature request / PR introduces a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to Connect Over USB Wifi Robots Not Appearing on Windows
5 participants