Add FixedFrame tf status in GlobalOptions #69

atharva-18 · 2020-11-29T12:01:26Z

Reference to issue #68

Added FixedFrame tf status.
Screenshots:

a. TF warn

b. TF error

c. Valid frame

Signed-off-by: atharva-18 <atharvapusalkar18@gmail.com>

Sarath18

This works for me. Please go through the following comments.

Sarath18 · 2020-12-02T16:40:39Z

ign_rviz_plugins/include/ignition/rviz/plugins/GlobalOptions.hpp

+  Q_PROPERTY(
+    QString tfStatus
+    READ getTfStatus
+    NOTIFY tfStatusChanged
+  )
+
+  /**
+   *  @brief TF status message
+   */
+  Q_PROPERTY(
+    QString tfStatusMessage
+    READ getTfStatusMessage
+    NOTIFY tfStatusMessageChanged
+  )
+
+  /**
+   *  @brief TF status color
+   */
+  Q_PROPERTY(
+    QString tfStatusColor
+    READ getTfStatusColor
+    NOTIFY tfStatusColorChanged
+  )


I believe instead of having 3 properties to read and notify the changes, we can have a custom Q_OBJECT class with these properties i.e status, message, and color along with methods to set these properties which are used in updateTfStatus.

This class can then be extended in the future to implement the tree view with the TF status of each frame.

Sarath18 · 2020-12-02T16:43:57Z

ign_rviz_plugins/src/rviz/plugins/GlobalOptions.cpp

@@ -104,6 +145,9 @@ bool GlobalOptions::eventFilter(QObject * _object, QEvent * _event)
    this->onRefresh();
  }

+  // Update tf status and message
+  this->updateTfStatus();


Currently, the updateTfStatus() function is being called on every Qt event received by this plugin. This results in a high TF status refresh rate which I don't think is necessary. This might also cause problems with other plugins using FrameManager due to the presence of locks.

I am not 100% sure on how to go about this, but updating the TF status alongside the periodic refresh of TF frames might be the right option. The periodic update of TF is currently under development in #62.

Signed-off-by: atharva-18 <atharvapusalkar18@gmail.com>

atharva-18 · 2021-01-20T17:11:01Z

@Sarath18 I have made the requested changes. The new TFStatus class holds the status, message, and color. I have moved the tfStatusChanged call inside the render event loop. However, the GlobalOptions.hpp file giving a code style divergence issue with Uncrustify. This is the error output:

5: Code style divergence in file 'include/ignition/rviz/plugins/GlobalOptions.hpp':
5: 
5: --- include/ignition/rviz/plugins/GlobalOptions.hpp
5: +++ include/ignition/rviz/plugins/GlobalOptions.hpp.uncrustify
5: @@ -43 +43 @@
5: - * 
5: + *
5: 
5: 1 files with code style divergence

Apologies for the delay.

ahcorde · 2021-01-20T17:13:19Z

5: Code style divergence in file 'include/ignition/rviz/plugins/GlobalOptions.hpp':
5: 
5: --- include/ignition/rviz/plugins/GlobalOptions.hpp
5: +++ include/ignition/rviz/plugins/GlobalOptions.hpp.uncrustify
5: @@ -43 +43 @@
5: - * 
5: + *
5: 
5: 1 files with code style divergence

There is space in the line 43, you should remove it

Signed-off-by: atharva-18 <atharvapusalkar18@gmail.com>

Sarath18 · 2021-01-23T15:49:33Z

ign_rviz_plugins/include/ignition/rviz/plugins/GlobalOptions.hpp

@@ -24,6 +24,7 @@
 #include <ignition/math/Color.hh>
 #include <QColor>
 #include <string>
+#include <vector>


Suggested change

#include <vector>

#include <memory>

#include <string>

#include <vector>

Sarath18 · 2021-01-23T16:02:09Z

ign_rviz_plugins/src/rviz/plugins/GlobalOptions.cpp

+  // Check for tf data
+  auto framePosition = std::find(_allFrames.begin(), _allFrames.end(), _fixedFrame);
+
+  if (!_allFrames.size()) {


Suggested change

if (!_allFrames.size()) {

if (_allFrames.empty()) {

Sarath18 · 2021-01-23T16:02:20Z

ign_rviz_plugins/res/qml/GlobalOptions.qml

+    RowLayout {
+      width: parent.width
+      spacing: 10
+
+      Label {
+        id: "status"
+        text: GlobalOptions.tfStatus.status
+        color: GlobalOptions.tfStatus.color
+      }
+
+      Label {
+        id: "message"
+        text: GlobalOptions.tfStatus.message
+        color: GlobalOptions.tfStatus.color
+      }
+    }


Can you look into the text wrapping issue?
The FixedFrame combo-box drop-down menu is not selectable when the plugin width < text width.

I have added elide property to the second Label element:

Default Expanded

Sarath18 · 2021-01-23T16:05:40Z

ign_rviz_plugins/src/rviz/plugins/GlobalOptions.cpp

+    if (currentState != 2) {
+      currentState = 2;
+      return true;
+    }


Are these checks really necessary?

These checks are performed to check whether the status has been changed. We can remove them and call emit tfStatusChanged() on every render event.

I think that will be better. Let's remove them for now. The updates on the render event will be helpful because:

The text Frame[<fixedframe>] does not exist text will be updated when the Frame is changed and the "currentState" is still the same. (In case of warning/error)

The implementation of the TFStatus will be modified in the future when the TF Status support for multiple frames is added.

Signed-off-by: atharva-18 <atharvapusalkar18@gmail.com>

Add FixedFrame status

6907089

Signed-off-by: atharva-18 <atharvapusalkar18@gmail.com>

Sarath18 reviewed Dec 2, 2020

View reviewed changes

atharva-18 added 2 commits January 20, 2021 21:34

Add TFStatus class

dfadedc

Signed-off-by: atharva-18 <atharvapusalkar18@gmail.com>

Remove whitespace in GlobalOptions.cpp

ad8361d

Signed-off-by: atharva-18 <atharvapusalkar18@gmail.com>

Remove space at line 43

0bbc95b

Signed-off-by: atharva-18 <atharvapusalkar18@gmail.com>

Sarath18 reviewed Jan 23, 2021

View reviewed changes

atharva-18 added 2 commits January 26, 2021 20:47

Remove currentState from TFStatus

70cd90e

Signed-off-by: atharva-18 <atharvapusalkar18@gmail.com>

Add elide property to message label

a047365

Signed-off-by: atharva-18 <atharvapusalkar18@gmail.com>

Sarath18 approved these changes Jan 27, 2021

View reviewed changes

Sarath18 requested a review from ahcorde January 27, 2021 06:22

Base automatically changed from master to main March 8, 2021 21:56

ahcorde merged commit 632a18a into gazebosim:main Aug 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add FixedFrame tf status in GlobalOptions #69

Add FixedFrame tf status in GlobalOptions #69

atharva-18 commented Nov 29, 2020

Sarath18 left a comment

Sarath18 Dec 2, 2020

Sarath18 Dec 2, 2020

atharva-18 commented Jan 20, 2021

ahcorde commented Jan 20, 2021

Sarath18 Jan 23, 2021

Sarath18 Jan 23, 2021

Sarath18 Jan 23, 2021

atharva-18 Jan 26, 2021

Sarath18 Jan 23, 2021

atharva-18 Jan 24, 2021

Sarath18 Jan 24, 2021

Add FixedFrame tf status in GlobalOptions #69

Add FixedFrame tf status in GlobalOptions #69

Conversation

atharva-18 commented Nov 29, 2020

Sarath18 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atharva-18 commented Jan 20, 2021

ahcorde commented Jan 20, 2021

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