Implement UI to change connection port and protocol #27
Conversation
| TextField( | ||
| controller: portController, | ||
| decoration: const InputDecoration(hintText: "Port"), | ||
| ), |
There was a problem hiding this comment.
Add a SizedBox between the text fields and the button, give it a height and experiment with values until you find something that looks nice (I think 8 would be a good starting point in this case)
| final comm = | ||
| MavlinkCommunication(MavlinkCommunicationType.tcp, '127.0.0.1', 14550); | ||
|
|
||
| final defaultCommunicationType = MavlinkCommunicationType.tcp; |
There was a problem hiding this comment.
I love the idea of having the the default params be constants! This feels like a good interim solution while your task is in progress. Maybe break these out into a separate file, preferably yaml but a Dart file with constant variables also works (this will be on the same level as main.dart)
| @@ -25,4 +32,11 @@ class HomePage extends StatelessWidget { | |||
| ), | |||
| ); | |||
| } | |||
|
|
|||
| void updateCommunicationParams(MavlinkCommunicationType connectionType, | |||
There was a problem hiding this comment.
Can this be a module instead?
| import 'package:flutter/material.dart'; | ||
| import 'package:imacs/modules/mavlink_communication.dart'; | ||
|
|
||
| const Map<MavlinkCommunicationType, String> mavCommTypes = { |
There was a problem hiding this comment.
A simplier way to do this is to use the .name attr of a MavlinkCommunicationType to get it's string representation:
final MavlinkCommunicationType _connectionType = MavlinkCommunicationType.tcp;
final String _connectionTypeStrRepr = _connectionType.name; // tcp| final MavlinkCommunicationType communicationType; | ||
| final String communicationAddress; | ||
| final int tcpPort; | ||
| final Function(MavlinkCommunicationType communicationType, |
There was a problem hiding this comment.
I don't really get the purpose of passing in a function into this widget, can you ping me on Discord to explain why this isn't just built in to the class.
|
|
||
| late MavlinkCommunicationType? selectedType = widget.communicationType; | ||
|
|
||
| var statusMsg = ""; |
There was a problem hiding this comment.
Always type your variables
String statusMsg = "";|
|
||
| @override | ||
| void dispose() { | ||
| addressController.dispose(); |
There was a problem hiding this comment.
Is there a reason we only dispose on of the TextEditingControllers
| controller: addressController, | ||
| decoration: const InputDecoration(hintText: "Address"), | ||
| ), | ||
| TextField( |
There was a problem hiding this comment.
Make this text field natural numbers only
|
|
||
| void main() { | ||
| group('Change Port/Protocol Widget', () { | ||
| testWidgets( |
There was a problem hiding this comment.
Add a test which actually changes the params and checks (I know this functionality isn't done yet but it's good to have tests early on)
|
Are there any updates on this? |
|
Hi @BalajiLeninrajan, sorry, just saw your comment now. Yes, I believe I implemented all the requested changes! I pinged you on the Discord post. Happy new year! |
No description provided.