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

Implement UI to change connection port and protocol #27

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
7 changes: 7 additions & 0 deletions flutter_app/.metadata
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
# This file should be version controlled and should not be manually edited.

version:
revision: "2663184aa79047d0a33a14a3b607954f8fdd8730"
revision: "2663184aa79047d0a33a14a3b607954f8fdd8730"
channel: "stable"

Expand All @@ -15,6 +16,12 @@ migration:
- platform: root
create_revision: 2663184aa79047d0a33a14a3b607954f8fdd8730
base_revision: 2663184aa79047d0a33a14a3b607954f8fdd8730
- platform: linux
create_revision: 2663184aa79047d0a33a14a3b607954f8fdd8730
base_revision: 2663184aa79047d0a33a14a3b607954f8fdd8730
- platform: macos
create_revision: 2663184aa79047d0a33a14a3b607954f8fdd8730
base_revision: 2663184aa79047d0a33a14a3b607954f8fdd8730
- platform: windows
create_revision: 2663184aa79047d0a33a14a3b607954f8fdd8730
base_revision: 2663184aa79047d0a33a14a3b607954f8fdd8730
Expand Down
7 changes: 7 additions & 0 deletions flutter_app/lib/defaults.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import 'package:imacs/modules/mavlink_communication.dart';

final class Defaults {
Copy link
Contributor

Choose a reason for hiding this comment

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

Make the class and file name more explicit

static const communicationType = MavlinkCommunicationType.tcp;
static const communicationAddress = '127.0.0.1';
static const tcpPort = 14550;
}
18 changes: 18 additions & 0 deletions flutter_app/lib/modules/change_port_protocol.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import 'dart:developer';

import 'package:imacs/modules/mavlink_communication.dart';

const String moduleName = "Change Port/Protocol";
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove special character / from module name


class ChangePortProtocol {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add doc strings to this file

final MavlinkCommunication comm;

ChangePortProtocol({required this.comm});

void updateCommParams(MavlinkCommunicationType connectionType,
String connectionAddress, int tcpPort) {
comm.updateConnectionParams(connectionType, connectionAddress, tcpPort);

log('[$moduleName] Communication params updated to: $connectionType, $connectionAddress, $tcpPort');
Copy link
Contributor

Choose a reason for hiding this comment

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

Good!

}
}
44 changes: 32 additions & 12 deletions flutter_app/lib/modules/mavlink_communication.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ class MavlinkCommunication {

final List<MissionItem> waypointQueue = [];

final MavlinkCommunicationType _connectionType;
MavlinkCommunicationType _connectionType;
String _connectionAddress;
int _tcpPort;

late Stream<Uint8List> _stream;
late SerialPort _serialPort;
Expand All @@ -28,17 +30,10 @@ class MavlinkCommunication {

MavlinkCommunication(MavlinkCommunicationType connectionType,
String connectionAddress, int tcpPort)
: _connectionType = connectionType {
switch (_connectionType) {
case MavlinkCommunicationType.tcp:
log('[$moduleName] Trying to start TCP connection');
_startupTcpPort(connectionAddress, tcpPort);
break;
case MavlinkCommunicationType.serial:
log('[$moduleName] Trying to start Serial connection');
_startupSerialPort(connectionAddress);
break;
}
: _connectionType = connectionType,
_connectionAddress = connectionAddress,
_tcpPort = tcpPort {
_startupPort(connectionType, connectionAddress, tcpPort);
}

_startupTcpPort(String connectionAddress, int tcpPort) async {
Expand All @@ -55,6 +50,20 @@ class MavlinkCommunication {
_stream = serialPortReader.stream;
}

_startupPort(MavlinkCommunicationType connectionType,
String connectionAddress, int tcpPort) {
switch (_connectionType) {
case MavlinkCommunicationType.tcp:
log('[$moduleName] Trying to start TCP connection');
_startupTcpPort(connectionAddress, tcpPort);
break;
case MavlinkCommunicationType.serial:
log('[$moduleName] Trying to start Serial connection');
_startupSerialPort(connectionAddress);
break;
}
}

_writeToTcpPort(MavlinkFrame frame) {
_tcpSocket.write(frame.serialize());
log('[$moduleName] Wrote a message to TCP Port. Frame ID: ${frame.componentId}');
Expand All @@ -81,7 +90,18 @@ class MavlinkCommunication {
}
}

void updateConnectionParams(MavlinkCommunicationType connectionType,
String connectionAddress, int tcpPort) {
_connectionType = connectionType;
_connectionAddress = connectionAddress;
_tcpPort = tcpPort;

_startupPort(connectionType, connectionAddress, tcpPort);
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good! It might be a good idea to close the current socket before starting up a new port.

}

MavlinkCommunicationType get connectionType => _connectionType;
String get connectionAddress => _connectionAddress;
int get tcpPort => _tcpPort;
Completer<void> get tcpSocketInitializationFlag =>
_tcpSocketInitializationFlag;
Socket get tcpSocket => _tcpSocket;
Expand Down
6 changes: 4 additions & 2 deletions flutter_app/lib/screens/home_screen.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'package:flutter/material.dart';
import 'package:imacs/defaults.dart';
import 'package:imacs/modules/mavlink_communication.dart';
import 'package:imacs/modules/get_drone_information.dart';
import 'package:imacs/widgets/drone_information_widget.dart';
Expand All @@ -8,8 +9,9 @@ class HomePage extends StatelessWidget {
HomePage({Key? key, required this.title}) : super(key: key);

final String title;
final comm =
MavlinkCommunication(MavlinkCommunicationType.tcp, '127.0.0.1', 14550);

late final comm = MavlinkCommunication(Defaults.communicationType,
Copy link
Contributor

Choose a reason for hiding this comment

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

pretty sure this doesn't have to be late

Defaults.communicationAddress, Defaults.tcpPort);

@override
Widget build(BuildContext context) {
Expand Down
93 changes: 93 additions & 0 deletions flutter_app/lib/widgets/change_port_protocol_widget.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import 'package:flutter/material.dart';
import 'package:imacs/modules/change_port_protocol.dart';
import 'package:imacs/modules/mavlink_communication.dart';

class PortProtocolChanger extends StatefulWidget {
Copy link
Contributor

Choose a reason for hiding this comment

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

make class match file name or vice versa

final MavlinkCommunication comm;
Copy link
Contributor

Choose a reason for hiding this comment

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

passing in comm here is unnecessary since you can do changePortProtocol.comm

final ChangePortProtocol changePortProtocol;

const PortProtocolChanger(
{super.key, required this.comm, required this.changePortProtocol});

@override
State<PortProtocolChanger> createState() => _PortProtocolChangerState();
}

class _PortProtocolChangerState extends State<PortProtocolChanger> {
late final TextEditingController addressController =
TextEditingController.fromValue(
TextEditingValue(text: widget.comm.connectionAddress));
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add trailing comma to closing ) of TextEditingValue for formatter

late final portController = TextEditingController.fromValue(
TextEditingValue(text: widget.comm.tcpPort.toString()));
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add trailing comma to closing ) of TextEditingValue for formatter


late MavlinkCommunicationType? selectedType = widget.comm.connectionType;

String statusMsg = "";
Copy link
Contributor

Choose a reason for hiding this comment

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

Not the most elegant way to do this but it works. (No changes needed)


@override
void dispose() {
addressController.dispose();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason we only dispose on of the TextEditingControllers

portController.dispose();
super.dispose();
}

@override
Widget build(BuildContext context) {
return Container(
decoration: BoxDecoration(
border: Border.all(color: Colors.black),
),
width: 400,
padding: const EdgeInsets.all(10.0),
child: Column(children: [
DropdownMenu<MavlinkCommunicationType>(
dropdownMenuEntries: MavlinkCommunicationType.values
.map<DropdownMenuEntry<MavlinkCommunicationType>>((entry) {
return DropdownMenuEntry(value: entry, label: entry.name);
}).toList(),
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add trailing comma to closing } of function arugment for formatter

initialSelection: widget.comm.connectionType,
onSelected: (MavlinkCommunicationType? newValue) {
setState(() {
selectedType = newValue;
});
}),
TextField(
controller: addressController,
decoration: const InputDecoration(hintText: "Address"),
),
TextField(
Copy link
Contributor

Choose a reason for hiding this comment

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

Make this text field natural numbers only

controller: portController,
decoration: const InputDecoration(hintText: "Port"),
),
Copy link
Contributor

Choose a reason for hiding this comment

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

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)

const SizedBox(height: 8),
ElevatedButton(
child: const Text("Update Connection Params"),
onPressed: () {
var tcpPort = int.tryParse(portController.text);

if (tcpPort == null) {
setState(() {
statusMsg = "Invalid port";
});
return;
} else if (tcpPort <= 0) {
setState(() {
statusMsg = "Port must be positive integer";
});
return;
}

widget.changePortProtocol.updateCommParams(
selectedType ?? widget.comm.connectionType,
addressController.text,
tcpPort);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add trailing comma to tcpPort for formatter

setState(() {
statusMsg = "Successfully changed";
});
},
),
Padding(
padding: const EdgeInsets.only(top: 5), child: Text(statusMsg)),
]));
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: split with commas for formatter

}
}
66 changes: 66 additions & 0 deletions flutter_app/test/widget/change_port_protocol_widget_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:imacs/modules/change_port_protocol.dart';
import 'package:imacs/modules/mavlink_communication.dart';
import 'package:imacs/widgets/change_port_protocol_widget.dart';

void main() {
group('Change Port/Protocol Widget', () {
testWidgets(
Copy link
Contributor

@BalajiLeninrajan BalajiLeninrajan Oct 13, 2024

Choose a reason for hiding this comment

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

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)

'PortProtocolChanger displays a dropdown menu, a button, and two input fields',
(WidgetTester tester) async {
MavlinkCommunication comm = MavlinkCommunication(
MavlinkCommunicationType.tcp, '127.0.0.1', 14550);

await tester.pumpWidget(MaterialApp(
home: Scaffold(
body: PortProtocolChanger(
comm: comm,
changePortProtocol: ChangePortProtocol(comm: comm),
),
),
));

expect(
find.byType(DropdownMenu<MavlinkCommunicationType>), findsOneWidget);
// Look for 3 TextFields because DropdownMenu also contains a TextField
expect(find.byType(TextField), findsNWidgets(3));
expect(find.byType(ElevatedButton), findsOneWidget);
});

testWidgets('PortProtocolChanger changes communication params',
(WidgetTester tester) async {
final comm = MavlinkCommunication(
MavlinkCommunicationType.tcp, '127.0.0.1', 14550);

await tester.pumpWidget(MaterialApp(
home: Scaffold(
body: PortProtocolChanger(
comm: comm,
changePortProtocol: ChangePortProtocol(comm: comm),
),
),
));

// Open the dropdown menu and select a mode
await tester.tap(find.byType(DropdownMenu<MavlinkCommunicationType>));
await tester.pumpAndSettle();
final serialFinder = find.descendant(
of: find.byType(DropdownMenu<MavlinkCommunicationType>),
matching: find.text('serial'),
);
await tester.tap(serialFinder.first);
await tester.pumpAndSettle();

final textFieldFinder = find.byType(TextField);
await tester.enterText(textFieldFinder.at(1), "127.0.1.0");
await tester.enterText(textFieldFinder.at(2), "14551");

// Press the change port/protocol button
await tester.tap(find.byType(ElevatedButton));
await tester.pumpAndSettle();

expect(find.text('Successfully changed'), findsOneWidget);
});
});
}