feat(app): Persist known robots to file-system when using new discovery #2065
Conversation
mcous
requested review from
Kadee80,
IanLondon,
b-cooper,
btmorr and
Laura-Danielle
mcous
added
feature
| @@ -1,7 +1,7 @@ | |||
| // simple script to advertise a MDNS service for local API | |||
| // TODO(mc, 2017-10-31): remove this file once API can advertise for itself | |||
There was a problem hiding this comment.
Technically should've updated this TODO. This file will now go away when new discovery is enabled by default
Codecov Report
@@ Coverage Diff @@
## edge #2065 +/- ##
==========================================
+ Coverage 33.09% 33.09% +<.01%
==========================================
Files 445 445
Lines 7125 7200 +75
==========================================
+ Hits 2358 2383 +25
- Misses 4767 4817 +50
Continue to review full report at Codecov.
|
app/src/discovery/index.js
Outdated
| } | ||
| } | ||
|
|
||
| return state | ||
| } | ||
|
|
||
| function flattenRobots (robots: Array<DiscoveredRobot> = []): RobotsMap { |
There was a problem hiding this comment.
flatten is a little misleading for array -> map IMO
There was a problem hiding this comment.
You're right, I'm gonna rename this to normalizeRobots as per redux convention. Don't know what was happening in my head when I named it "flatten"
app/src/discovery/index.js
Outdated
| } | ||
| } | ||
|
|
||
| return state | ||
| } | ||
|
|
||
| function flattenRobots (robots: Array<DiscoveredRobot> = []): RobotsMap { | ||
| return robots.reduce( | ||
| (robotsMap, robot) => ({ |
There was a problem hiding this comment.
ever since it made Flow crash on me a few months ago (facebook/flow#5831) I've been paranoid about unannotated accumulators in reduce -- you might wanna robotsMap: RobotsMap. AFAIK it's safe to leave the value (robot here) without annotation, but not the accumulator arg in the reduce cb.
There was a problem hiding this comment.
Noted. Already touching this file; will add type annotations for safety
|
A few comments from Windows 10: Took a few refreshes to see the robot USB (IPv4) -> could be what that customer was seeing earlier today. |
overview
This PR adds robot persistence to file to the app and wires up the necessary state. It also includes some tweaks to various values and configurations based on how discovery behaves in the real world.
changelog
review requests
Try this out on your machine!
make -C discovery-client(Re-build discovery client)make -C app dev OT_APP_DISCOVERY__ENABLED=1discovery.robotsByNamestate is prepopulated at app launch from fileopentrons-devKnown issues: