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

RSDK-7769, RSDK-7770 Add AppRobotClient and LogOutput dial option #233

Merged
merged 7 commits into from
Jun 25, 2024

Conversation

benjirewis
Copy link
Member

@benjirewis benjirewis commented Jun 17, 2024

RSDK-7769
RSDK-7770

Adds a wrapper for app's robot service and implements only the log method. Adds a logOutput option to dial options to allow specifying a custom log output.

To be used in PR-LINK-TODO to allow the mobile app the send connection establishment logs to app.viam.com.

@benjirewis benjirewis changed the title RSDK-7769, RSDK-7770 Add AppRobotClient and LogOuput dial option RSDK-7769, RSDK-7770 Add AppRobotClient and LogOutput dial option Jun 17, 2024
@benjirewis benjirewis marked this pull request as ready for review June 17, 2024 16:45
@benjirewis benjirewis requested a review from a team as a code owner June 17, 2024 16:45
@benjirewis benjirewis requested a review from stuqdog June 17, 2024 16:45
Logger newDialLogger(LogOutput? output) {
// Use a SimplePrinter, as flutter dial logs from the RC app are sent to app.viam.com,
// and pretty-printed logs are overly formatted.
return Logger(output: output, printer: SimplePrinter(colors: false));
Copy link
Member Author

Choose a reason for hiding this comment

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

If you're curious what the logs look like on app.

For connection success (latest->oldest):

6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: chose host local candidate with IP 127.0.0.1:59852 @ 2024-06-17 12:10:05.493
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: chose host remote candidate with IP 127.0.0.1:49963 @ 2024-06-17 12:10:05.493
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: 10 call updates to the signaling server were made
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: max call update took 0:00:00.097045
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: average call update took 0:00:00.090000
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: all ICE candidates gathered in 0:00:00.661795
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: call update 7 took 0:00:00.097045
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: call update 9 took 0:00:00.091183
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: call update 1 took 0:00:00.092558
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: call update 6 took 0:00:00.091111
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: call update 8 took 0:00:00.089822
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: call update 0 took 0:00:00.091249
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: call update 5 took 0:00:00.089269
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: call update 2 took 0:00:00.088691
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: call update 3 took 0:00:00.089009
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: call update 4 took 0:00:00.086586
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: adding remote ICE candidate of candidate:2878742611 1 udp 2130706431 127.0.0.1 49963 typ host
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: adding remote ICE candidate of candidate:1352458247 1 udp 1694498815 71.167.222.76 62792 typ srflx raddr 0.0.0.0 rport 62792
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: WebRTC connection made in 0:00:00.522645
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: making call update 9 to the signaling server
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: candidate candidate:2459122801 1 tcp 1518157055 ::1 61490 typ host tcptype passive generation 0 ufrag wNPI network-id 4 gathered
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: candidate candidate:617964620 1 tcp 1518083839 127.0.0.1 61488 typ host tcptype passive generation 0 ufrag wNPI network-id 3 gathered
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: making call update 4 to the signaling server
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: making call update 6 to the signaling server
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: making call update 8 to the signaling server
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: candidate candidate:2901833412 1 tcp 1518285567 fd8f:df19:cb63:e74b:d3:4a65:9d96:f99b 61489 typ host tcptype passive generation 0 ufrag wNPI network-id 2 network-cost 10 gathered
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: making call update 5 to the signaling server
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: candidate candidate:103336020 1 tcp 1518214911 10.1.9.95 61487 typ host tcptype passive generation 0 ufrag wNPI network-id 1 network-cost 10 gathered
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: candidate candidate:3613791393 1 udp 41820159 34.203.251.37 53375 typ relay raddr 71.167.222.76 rport 62973 generation 0 ufrag wNPI network-id 1 network-cost 10 gathered
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: making call update 7 to the signaling server
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: making call update 2 to the signaling server
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: candidate candidate:1511687892 1 udp 2122063615 127.0.0.1 59852 typ host generation 0 ufrag wNPI network-id 3 gathered
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: making call update 3 to the signaling server
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: making call update 1 to the signaling server
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: candidate candidate:3965509353 1 udp 2122136831 ::1 52084 typ host generation 0 ufrag wNPI network-id 4 gathered
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: candidate candidate:3526988892 1 udp 2122265343 fd8f:df19:cb63:e74b:d3:4a65:9d96:f99b 54348 typ host generation 0 ufrag wNPI network-id 2 network-cost 10 gathered
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: candidate candidate:2492806202 1 udp 1685987071 71.167.222.76 62973 typ srflx raddr 10.1.9.95 rport 62973 generation 0 ufrag wNPI network-id 1 network-cost 10 gathered
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: candidate candidate:2028415692 1 udp 2122194687 10.1.9.95 62973 typ host generation 0 ufrag wNPI network-id 1 network-cost 10 gathered
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: making call update 0 to the signaling server
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: adding remote ICE candidate of candidate:2323467334 1 udp 2130706431 10.1.9.95 50491 typ host
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: created peer connection and channels in 0:00:00.009970
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: connected to signaling in 0:00:00.129933
6/17/2024, 12:10:05 PM debug flutter-sdk [D] STATS: authentication took 0:00:00.129197
6/17/2024, 12:10:05 PM debug flutter-sdk [D] Authenticated to address: app.viam.com:443
6/17/2024, 12:10:04 PM debug flutter-sdk [D] STATS: dial function took 0:00:00.002614
6/17/2024, 12:10:04 PM debug flutter-sdk [D] Authenticating to address: app.viam.com:443, for entity: foo-main.h9kzybd9wn.viam.cloud
6/17/2024, 12:10:04 PM debug flutter-sdk [D] Connecting to signaling server: app.viam.com
6/17/2024, 12:10:04 PM debug flutter-sdk [D] Auth entity is empty, setting to host: foo-main.h9kzybd9wn.viam.cloud
6/17/2024, 12:10:04 PM debug flutter-sdk [D] Dialing direct GRPC to app.viam.com
6/17/2024, 12:10:04 PM debug flutter-sdk [D] Dialing WebRTC to foo-main.h9kzybd9wn.viam.cloud
6/17/2024, 12:10:04 PM info flutter-sdk [I] Connecting to address foo-main.h9kzybd9wn.viam.cloud

For connection failure (latest->oldest):

6/17/2024, 12:09:01 PM error flutter-sdk [E] Could not authenticate to address: foo-main.h9kzybd9wn.viam.cloud:443 ERROR: gRPC Error (code: 14, codeName: UNAVAILABLE, message: Error connecting: SocketException: Connection refused (OS Error: Connection refused, errno = 61), address = foo-main.h9kzybd9wn.viam.cloud, port = 61303, details: null, rawResponse: null, trailers: {})
6/17/2024, 12:09:01 PM debug flutter-sdk [D] Authenticating to address: foo-main.h9kzybd9wn.viam.cloud:443, for entity: foo-main.h9kzybd9wn.viam.cloud
6/17/2024, 12:09:01 PM debug flutter-sdk [D] Dialing direct GRPC to foo-main.h9kzybd9wn.viam.cloud
6/17/2024, 12:09:01 PM info flutter-sdk [I] Could not connect via WebRTC, attempting direct gRPC connection ERROR: gRPC Error (code: 4, codeName: DEADLINE_EXCEEDED, message: context deadline exceeded, details: [], rawResponse: null, trailers: {})
6/17/2024, 12:08:51 PM debug flutter-sdk [D] STATS: created peer connection and channels in 0:00:00.009587
6/17/2024, 12:08:51 PM debug flutter-sdk [D] STATS: authentication took 0:00:00.130151
6/17/2024, 12:08:51 PM debug flutter-sdk [D] STATS: connected to signaling in 0:00:00.131668
6/17/2024, 12:08:51 PM debug flutter-sdk [D] Authenticated to address: app.viam.com:443
6/17/2024, 12:08:51 PM debug flutter-sdk [D] STATS: dial function took 0:00:00.013401
6/17/2024, 12:08:51 PM debug flutter-sdk [D] Authenticating to address: app.viam.com:443, for entity: foo-main.h9kzybd9wn.viam.cloud
6/17/2024, 12:08:51 PM debug flutter-sdk [D] Dialing direct GRPC to app.viam.com
6/17/2024, 12:08:51 PM debug flutter-sdk [D] Auth entity is empty, setting to host: foo-main.h9kzybd9wn.viam.cloud
6/17/2024, 12:08:51 PM debug flutter-sdk [D] Dialing WebRTC to foo-main.h9kzybd9wn.viam.cloud
6/17/2024, 12:08:51 PM debug flutter-sdk [D] Connecting to signaling server: app.viam.com
6/17/2024, 12:08:51 PM info flutter-sdk [I] Connecting to address foo-main.h9kzybd9wn.viam.cloud

Copy link
Member

@cheukt cheukt left a comment

Choose a reason for hiding this comment

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

LGTM from a netcode perspective

final String message = '${event.lines.join('\n')}\n';

final LogEntry entry =
LogEntry(host: '$partId-$flutterSdkLoggerName', level: level, time: protoTs, loggerName: flutterSdkLoggerName, message: message);
Copy link
Member

Choose a reason for hiding this comment

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

should we allow the host/loggerName to be user defined?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah probably a good idea 👍🏻 .

Copy link
Member

@stuqdog stuqdog left a comment

Choose a reason for hiding this comment

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

Generally looks good to me, though I'm not the greatest expert on dart. I wonder if we want at least a simple test however?

Copy link
Member Author

@benjirewis benjirewis left a comment

Choose a reason for hiding this comment

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

Added a test for custom dial log output and a test for the new log method on the AppRobotClient.

@@ -11,25 +11,27 @@ import 'package:grpc/src/client/channel.dart' as _i5;
import 'package:grpc/src/client/connection.dart' as _i2;
import 'package:grpc/src/client/method.dart' as _i7;
import 'package:mockito/mockito.dart' as _i1;
import 'package:viam_sdk/src/gen/app/data/v1/data.pb.dart' as _i13;
Copy link
Member Author

@benjirewis benjirewis Jun 20, 2024

Choose a reason for hiding this comment

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

Lots of changes to this auto-generated mocks file after adding the new MockSpec above and running make build_mocks. Some of the changes add a new MockAppRobotServiceClient class; most of the changes are due to changed imports and missing methods on other mocks such as Shutdown.

@benjirewis benjirewis requested a review from stuqdog June 20, 2024 14:30
@benjirewis
Copy link
Member Author

@njooma not sure who's a required review on this; will wait for you and @clintpurser 🫡 .

@@ -17,7 +17,13 @@ import '../utils.dart';
import 'grpc/grpc_or_grpcweb_channel.dart';
import 'web_rtc/web_rtc_client.dart';

final _logger = Logger(printer: PrettyPrinter(printTime: true));
Logger newDialLogger(LogOutput? output) {
Copy link
Member

Choose a reason for hiding this comment

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

can you make this private as well please?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep.

@benjirewis benjirewis requested a review from njooma June 24, 2024 17:11
@njooma njooma merged commit c832369 into viamrobotics:main Jun 25, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants