-
Notifications
You must be signed in to change notification settings - Fork 2
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
Control Hub Module #68
base: main
Are you sure you want to change the base?
Conversation
The Control Hub must be running a RC app with this commit in order to properly show up in the list |
I currently have it set to timeout after 1 second when checking if a control hub is present. If there's not a control hub, this adds an extra second to the list command. We may want to tune this timeout in the future (or better yet, add it as a parameter to the discovery method) |
A WiFi-connected Control Hub will not have a serialNumber, so it isn't conformant to the ParentRevHub interface. I feel like conceptually it is a parent, and can indeed have children, so we'll need a better solution. |
It may be good to create a ParentUsbHub interface that has the serialNumber, and add a isParentUsbHub method? |
We could add the serial number to rcInfo.json using |
8c62f05
to
a3712d3
Compare
One thing to note about this PR is that other PRs change the expansion hub interface. When creating stubs for commands, I preferred the modified methods (essentially what the interface should look like when all PRs are merged). This means that several PRs have changes this one will need. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a few more comments, but there's still unresolved ones.
README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current wording implies that adbkit
is listed as an actual optional dependency, which is not the case.
packages/control-hub/package.json
Outdated
"@rev-robotics/rev-hub-core": "*" | ||
}, | ||
"peerDependencies": { | ||
"get-port": "6.x" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get-port is no longer used by this package, right? We just want to document how to use it with adbkit to set up port forwarding.
import getPort from "get-port"; | ||
import { ControlHub } from "./ControlHub.js"; | ||
|
||
export async function openConnectedControlHub(): Promise<ControlHub | undefined> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be named something like openControlHub
, and accept an IP address and httpPort
, with default values of 192.168.43.1
and 8080
.
See also WIP changes in this commit: e0b79e9 |
cf6d277
to
230c40d
Compare
This PR needs a non-production version of the RC app.
This PR adds a control-hub module that allows the user to control a REV control hub via WiFi or USB.