Allow specifying device-vmservice-port and host-vmservice-port#44027
Allow specifying device-vmservice-port and host-vmservice-port#44027jonahwilliams merged 9 commits intoflutter:masterfrom
Conversation
|
Still not ready I guess, my bad! |
| /// If no port is set, returns null. | ||
| int get hostVmservicePort { | ||
| if (!_usesPortOption || | ||
| argResults['observatory-port'] == null || |
There was a problem hiding this comment.
Not sure if this is the right place, but a check that only one of --observatory-port or --host-vmservice-port is set would be good.
| 'discovered.', | ||
| ); | ||
| argParser.addOption('host-vmservice-port', | ||
| help: 'Listen to the given port for a vmservice connection.\n' |
There was a problem hiding this comment.
Maybe: 'When a device-side vmservice port is forwarded to a host-side port, use this value as the host port.'
| this, | ||
| ipv6, | ||
| debuggingOptions.observatoryPort, | ||
| debuggingOptions.hostVmservicePort, |
There was a problem hiding this comment.
The device port could also be used to filter results from an mdns query. It looks like currently MDnsObservatoryDiscovery will throw a tool exit if there is more than one result, asking for an appId so that it can refine the mdns query, but another way to distinguish would be to pass the device port to the engine and then filter on it. Maybe add a TODO.
| getLogReader(), | ||
| portForwarder: portForwarder, | ||
| hostPort: debuggingOptions.observatoryPort, | ||
| hostPort: debuggingOptions.hostVmservicePort, |
There was a problem hiding this comment.
How do we format Vmservice in other places? Should we do VMService, VmService, or keep Vmservice?
There was a problem hiding this comment.
Updated to consistently use VmService
|
|
||
| Future<Uri> getObservatoryUri(String applicationId, Device device, [bool usesIpv6 = false, int observatoryPort]) async { | ||
| final MDnsObservatoryDiscoveryResult result = await query(applicationId: applicationId); | ||
| Future<Uri> getObservatoryUri(String applicationId, Device device, {bool usesIpv6 = false, int hostVmservicePort, int deviceVmservicePort}) async { |
| Future<Uri> getObservatoryUri(String applicationId, Device device, [bool usesIpv6 = false, int observatoryPort]) async { | ||
| final MDnsObservatoryDiscoveryResult result = await query(applicationId: applicationId); | ||
| Future<Uri> getObservatoryUri(String applicationId, Device device, {bool usesIpv6 = false, int hostVmservicePort, int deviceVmservicePort}) async { | ||
| final MDnsObservatoryDiscoveryResult result = await query(applicationId: applicationId, deviceVmservicePort: deviceVmservicePort); |
| } | ||
| final int localPort = observatoryPort | ||
| ?? await device.portForwarder.forward(devicePort); | ||
| final int localPort = await device.portForwarder.forward(devicePort, hostPort: hostVmservicePort); |
There was a problem hiding this comment.
In some other places, the name actualHostPort is used, which I think might be a little more clear.
Description
--host-vmservice-port supersedes --observatory-port which is now soft-deprecated. --device-vmservice-port applies a filter to the observatory discovery process.
WIP due to phrasing and tests