-
Notifications
You must be signed in to change notification settings - Fork 2
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
Rtl button #28
base: main
Are you sure you want to change the base?
Rtl button #28
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far so good! You need to rewrite some stuff, look at the implementation of the mode changer, that is the widget that's most similar to yours
/// | ||
/// @return A MissionItem representing the reutrn to launch command. | ||
|
||
MissionItem returnToLaunch(int sequence, int systemID, int componentID, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I didn't read the docs when I approved this. You need to use a Command Long for this, check set_mode_constructor.dart, that should be similar.
final MissionItem returnToLaunchConstructor; | ||
|
||
// Requires both the constructor (Mission Item) and comm | ||
ReturnToLaunch({required this.comm, required this.returnToLaunchConstructor}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can call the command constructor within the module (look at change_drone_mode for an example)
await comm.tcpSocketInitializationFlag.future; | ||
} | ||
|
||
var frame = MavlinkFrame.v2( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part should be done in the command constructor
@@ -0,0 +1,40 @@ | |||
import 'package:dart_mavlink/dialects/common.dart'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
expect(find.text("Return To Launch"), findsOneWidget); | ||
}); | ||
|
||
testWidgets("Button sends MavLink command", (WidgetTester tester) async {}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you need help with this test lmk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for taking a while. Your code lgtm! As for you last test case I left a comment, fixing it will involve some work changing your widget and honestly atm it's kinda optional since none of the front end is final as of now. Ping me on Discord if you have any questions
await tester.tap(find.byType(ReturnToLaunchButton)); | ||
await tester.pumpAndSettle(); | ||
|
||
expect(find.text("Return to Launch Command Sent"), findsOneWidget); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There isn't really a perfect way to check this - UI is weird :'). I'm guess you copied this over from the test for set drone more, this works over there since when the drone mode is changed, the text displayed also changes (in the widget not the log). Low key this test isn't that important since the actual widget part is subject to change. If you still want to work on this try adding a way for you module to communicate with the rest of the program that the command has been sent.
Widget build(BuildContext context) { | ||
return ElevatedButton( | ||
onPressed: () async { | ||
bool result = await widget.returnToLaunchCommand |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason the text of you button is not changing is that this async code never finished executing. This is actually to be expected since part of the async code is the completer for TCP initialization, which will never complete unless connected to the drone.
onPressed: () async { | ||
bool result = await widget.returnToLaunchCommand | ||
.returnNoQueue(widget.systemID, widget.componentID); | ||
if (true) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary if statement
String buttonLabel = "Return to Launch"; | ||
|
||
void updateButton() { | ||
setState(() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of updating the text of the button, I think it will be better to have a temporary SnackBar appear
ReturnToLaunch({required this.comm}); | ||
|
||
// Skips the queues and forces the drone to return | ||
Future returnNoQueue(int systemID, int componentID) async { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make return type explicit Future<bool>
Implemented code for a return to launch function