[RSDK-8920] Add DiscoverComponents to robot client #274
Conversation
| @@ -5,17 +5,19 @@ import 'package:grpc/grpc_connection_interface.dart'; | |||
| import 'package:logger/logger.dart'; | |||
|
|
|||
| import '../gen/common/v1/common.pb.dart'; | |||
| import '../gen/robot/v1/robot.pbgrpc.dart'; | |||
There was a problem hiding this comment.
"Real" non-proto update changes occur in this file
| @@ -5,17 +5,19 @@ import 'package:grpc/grpc_connection_interface.dart'; | |||
| import 'package:logger/logger.dart'; | |||
|
|
|||
| import '../gen/common/v1/common.pb.dart'; | |||
| import '../gen/robot/v1/robot.pbgrpc.dart'; | |||
| import '../gen/google/protobuf/struct.pb.dart'; | |||
| import '../gen/robot/v1/robot.pbgrpc.dart' as rpb; | |||
There was a problem hiding this comment.
import aliasing robot pb to avoid identifier collision between the new DiscoveryQuery in the SDK, and the pb DiscoveryQuery
| /// {@category Viam SDK} | ||
| /// Represents a discovery query in the SDK to query for discoverable components. | ||
| class DiscoveryQuery { | ||
| final String subtype; | ||
| final String model; | ||
| final Map<String, dynamic> extra; | ||
|
|
||
| DiscoveryQuery({required this.subtype, required this.model, Map<String, dynamic>? extra}) : extra = extra ?? {}; | ||
|
|
||
| Struct get extraStruct => extra.toStruct(); | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't think this class should exist and that people should just use the DiscoveryQuery proto directly. This doesn't add any additional functionality or ergonomics to the developer IMO
There was a problem hiding this comment.
If we don't make this class, how do we import rpb's proto DiscoveryQuery in our app? I tried using import 'package:viam_sdk/viam_sdk.dart'; in my test app, and it doesn't seem to import proto methods and classes.
There was a problem hiding this comment.
lib/main.dart:53:19: Error: The method 'DiscoveryQuery' isn't defined for the class '_MyHomePageState'.
- '_MyHomePageState' is from 'package:sean_flutter_app/main.dart' ('lib/main.dart').
Try correcting the name to the name of an existing method, or defining a method named 'DiscoveryQuery'.
There was a problem hiding this comment.
I would be in favor of DRY-ing this data struct. But is there a convenient way to import proto classes into user apps?
There was a problem hiding this comment.
You can import it from viam_sdk/protos/... or you can add it as a typedef here similar to how we do it for CloudMetadata a few lines above (the preferred way IMO)
There was a problem hiding this comment.
Got it let me check out the latter way
There was a problem hiding this comment.
I tried the latter way, I had a hard time figuring out how to initialize the extra pb struct. Ended up importing import 'package:viam_sdk/src/utils.dart'; for the map toStruct helper logic. This seems like bad practice though since utils.dart seems purposefully excluded as an export from viam_sdk.dart.
Open to ideas on how to make it easier to use extra without the Struct get extraStruct => extra.toStruct(); utility in the DiscoveryQuery first draft.
https://viam.atlassian.net/browse/RSDK-8920
extraFirst PR in Flutter SDK and first time writing code Dart; appreciate tips and best practice comments on top of correctness