Sv 13 driving mode switching #12

TomyangSydney · 2018-09-08T02:41:21Z

SV-13 is about driving mode switching. Key 'A' represents front wheel steering, Key 'B' represents crab steering , Keys 'D' represent four wheel steering. When certain keys are pressed, certain block on GUI should becomes red.

I add a publisher to the GUI so that when key pressing is detected, the variable 'a' should increase by 1 and send through topic 'ros_driving_mode_switching'. The value of a should be visualized on the screen

…en A/S/D pressed the relavent mode will be red #time 8h

create a driving-mode-switching class with a publisher to report keypress event

wmpmiles · 2018-09-15T01:44:20Z

@TomyangSydney Master has been updated, please re-merge and resolve conflicts.

…. Some bugs may inside camera switching

sajidanower23 · 2018-09-20T13:00:20Z

src/ros_video_components/src/ros_driving_mode_switching.cpp

@@ -0,0 +1,86 @@
+#include "ros_video_components/ros_driving_mode_switching.hpp"
+
+#define RECT_X 0


These #defines are not being used anywhere, are they? I reckon get rid of all of these unnecessary #defines

sajidanower23 · 2018-09-20T13:01:08Z

src/ros_video_components/src/ros_driving_mode_switching.cpp

+    }
+}
+
+


Get rid of some of the unnecessary blank lines maybe?

sajidanower23 · 2018-09-20T13:01:32Z

src/ros_video_components/include/ros_video_components/ros_driving_mode_switching.hpp

+        int data; //the signal strength in decibels
+};
+
+


Plenty of unnecessary blank lines 😄

sajidanower23 · 2018-09-20T13:03:19Z

src/ros_video_components/src/ros_driving_mode_switching.cpp

+
+
+
+void ROS_Driving_Mode_Switching::receive_signal(const std_msgs::Float32::ConstPtr & msg){


Why is this function called receive_signal? Should it not be something like receive_command?
The reason I called this receive_signal in my Signal Strength Indicator code is that this function was being used to receive the value of signal strength

sajidanower23 · 2018-09-20T13:05:06Z

src/ros_video_components/src/owr_ros_components.cpp

@@ -14,9 +14,13 @@
 #include "ros_video_components/ros_joystick_listener.hpp"
 #include "ros_video_components/ros_drive_mode.hpp"

+//add the new include directory
+#include "ros_video_components/ros_driving_mode_switching.hpp"


Get rid of the comment above and make this #include line 16

sajidanower23 · 2018-09-20T13:07:11Z

src/ros_video_components/include/ros_video_components/ros_driving_mode_switching.hpp

+ * ROS Package: owr_qtgui
+ * @description: This code generates a publisher called signal_pub, which will make a variable called a increase by 1
+ * when keyA/S/D are pressed and print it out
+ * @copyright: This code is released under the MIT [GPL for embeded] License. Copyright BLUEsat UNSW, 2017


Should be 2018 instead of 2017. Also, get rid of the [GPL for embedded] part

sajidanower23 · 2018-09-20T13:08:38Z

src/ros_video_components/include/ros_video_components/ros_driving_mode_switching.hpp

+/**
+ * Date Started: 28/07/18
+ * @author: [Linchuan YANG]
+ * @authors: (Editors) [Editor 1], [Editor 2]


Make @authors @editors and keep the right hand side empty

sajidanower23 · 2018-09-20T13:09:17Z

src/gui/resources/main_window.qml

}
+

Unnecessary blank lines

sajidanower23 · 2018-09-20T13:10:46Z

src/gui/resources/main_window.qml

+                Keys.onPressed: {
+
+
+                    switch(event.key){


De-indent this line a bit

sajidanower23 · 2018-09-20T13:11:46Z

qt-gui.workspace.user.5a83cd3

@@ -0,0 +1,246 @@
+<?xml version="1.0" encoding="UTF-8"?>


Run the command git rm qt-gui.workspace.user.5a83cd3 to get rid of this file. I don't think this file should be committed to git.

…rmat

…-qtgui into SV-13-Driving-Mode-Switching

…ch the order of camera switching and driving mode switching in the qml file

sajidanower23

I can confirm that the visuals work, but I believe the drive-mode does not actually change.

sajidanower23 · 2018-11-24T03:20:47Z

src/ros_video_components/src/ros_driving_mode_switching.cpp

+    QQuickPaintedItem(parent),
+    topic_value("/rover/driving_mode_switching/"),
+    ros_ready(false),
+    a(0),


Suggested change

a(0),

sajidanower23 · 2018-11-24T03:21:15Z

src/ros_video_components/src/ros_driving_mode_switching.cpp

+    msg.data = 1000;
+    drive_mode_pub.publish(msg);
+    //when the key 'A/S/D'pressed, a should add 1
+    a++;


Suggested change

a++;

sajidanower23 · 2018-11-24T03:21:25Z

src/ros_video_components/src/ros_driving_mode_switching.cpp

+    drive_mode_pub.publish(msg);
+    //when the key 'A/S/D'pressed, a should add 1
+    a++;
+    ROS_INFO("value is %d",a);


Suggested change

ROS_INFO("value is %d",a);

sajidanower23 · 2018-11-24T03:22:13Z

src/ros_video_components/src/ros_driving_mode_switching.cpp

+
+void ROS_Driving_Mode_Switching::receive_drive_mode_command(const std_msgs::Float32::ConstPtr & msg){
+    data = msg->data;
+    ROS_INFO("Received signal message");


Suggested change

ROS_INFO("Received signal message");

ROS_INFO("Received update for drive mode");

sajidanower23 · 2018-11-24T03:22:48Z

src/ros_video_components/src/ros_driving_mode_switching.cpp

+}
+
+void ROS_Driving_Mode_Switching::set_topic(const QString & new_value) {
+    ROS_INFO("set_topic");


Unnecessary print, I think.

sajidanower23 · 2018-11-24T03:23:23Z

src/ros_video_components/src/ros_driving_mode_switching.cpp

+   std_msgs::Float32 msg;
+    msg.data = 1000;
+    drive_mode_pub.publish(msg);
+    //when the key 'A/S/D'pressed, a should add 1


Is this variable necessary? It also risks an unlikely case of integer overflow

sajidanower23 · 2018-11-24T03:24:59Z

src/ros_video_components/include/ros_video_components/ros_video_component.hpp

@@ -19,7 +19,7 @@
 #include <ros/ros.h>
 #include <image_transport/image_transport.h>
 #include <sensor_msgs/Image.h>
-#include "owr_messages/activeCameras.h"
+//#include "owr_messages/activeCameras.h"


Suggested change

//#include "owr_messages/activeCameras.h"

sajidanower23 · 2018-11-24T03:25:12Z

src/ros_video_components/include/ros_video_components/ros_driving_mode_switching.hpp

+        ros::Publisher drive_mode_pub;
+        QString topic_value;
+        bool ros_ready;
+        int a;


Unnecessary variable

sajidanower23 · 2018-11-24T03:25:44Z

src/gui/src/main_application.cpp

@@ -51,11 +56,11 @@ void Main_Application::run() {
    // setup the timer
    ROSTimer * stopwatch = this->rootObjects()[0]->findChild<ROSTimer *>(QString("timerDisplay"));
    // the following code section is for debugging
-    /*
+


Why did you comment the stopwatch in?

sajidanower23 · 2018-11-24T03:27:07Z

qt-gui.workspace.user

@@ -63,7 +63,13 @@
  <valuemap type="QVariantMap">
   <value type="QString" key="ProjectExplorer.ProjectConfiguration.DefaultDisplayName">Desktop</value>
   <value type="QString" key="ProjectExplorer.ProjectConfiguration.DisplayName">Desktop</value>
+<<<<<<< HEAD


You may want to resolve the merge conflicts in this file.

TomyangSydney added 3 commits May 26, 2018 13:51

SV-13 #comment Switching Mode Indicator Finished SV-13 26/05/18 8h Wh…

b3b21a7

…en A/S/D pressed the relavent mode will be red #time 8h

widget responding to keypress SV-13 #time 40h

363c4cd

create a driving-mode-switching class with a publisher to report keypress event

SV-13 change the code style according to the style guide line

bcd3adb

TomyangSydney requested a review from wmpmiles September 8, 2018 02:41

Merge branch 'master' into SV-13-Driving-Mode-Switching

e8030bf

Fix some bugs in SV-13-Driving-Mode-Switching when merge master to it…

2c93bd4

…. Some bugs may inside camera switching

TomyangSydney force-pushed the SV-13-Driving-Mode-Switching branch from e8030bf to 2c93bd4 Compare September 15, 2018 05:43

sajidanower23 requested changes Sep 20, 2018

View reviewed changes

sajidanower23 assigned TomyangSydney Sep 20, 2018

TomyangSydney and others added 3 commits September 22, 2018 12:47

Make some changes to the SV-13 code to improve the readability and fo…

503295b

…rmat

Merge branch 'SV-13-Driving-Mode-Switching' of github.com:bluesat/owr…

829d845

…-qtgui into SV-13-Driving-Mode-Switching

SV-13 Driving Mode Switching, fix some problems in the qml file. Swit…

051b2e3

…ch the order of camera switching and driving mode switching in the qml file

sajidanower23 requested changes Nov 24, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sv 13 driving mode switching #12

Sv 13 driving mode switching #12

TomyangSydney commented Sep 8, 2018

wmpmiles commented Sep 15, 2018

sajidanower23 Sep 20, 2018

sajidanower23 Sep 20, 2018

sajidanower23 Sep 20, 2018 •

edited

Loading

sajidanower23 Sep 20, 2018

sajidanower23 Sep 20, 2018

sajidanower23 Sep 20, 2018

sajidanower23 Sep 20, 2018

sajidanower23 Sep 20, 2018

sajidanower23 Sep 20, 2018

sajidanower23 Sep 20, 2018

sajidanower23 left a comment

sajidanower23 Nov 24, 2018

sajidanower23 Nov 24, 2018

sajidanower23 Nov 24, 2018

sajidanower23 Nov 24, 2018

sajidanower23 Nov 24, 2018

sajidanower23 Nov 24, 2018

sajidanower23 Nov 24, 2018

sajidanower23 Nov 24, 2018

sajidanower23 Nov 24, 2018

sajidanower23 Nov 24, 2018

		@@ -0,0 +1,86 @@
		#include "ros_video_components/ros_driving_mode_switching.hpp"

		#define RECT_X 0




		void ROS_Driving_Mode_Switching::receive_signal(const std_msgs::Float32::ConstPtr & msg){

	ROS_INFO("Received signal message");
	ROS_INFO("Received update for drive mode");

Sv 13 driving mode switching #12

Are you sure you want to change the base?

Sv 13 driving mode switching #12

Conversation

TomyangSydney commented Sep 8, 2018

wmpmiles commented Sep 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sajidanower23 Sep 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sajidanower23 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sajidanower23 Sep 20, 2018 •

edited

Loading