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

Plot plugin #456

Open
francocipollone opened this issue Sep 15, 2021 · 8 comments
Open

Plot plugin #456

francocipollone opened this issue Sep 15, 2021 · 8 comments

Comments

@francocipollone
Copy link
Collaborator

francocipollone commented Sep 15, 2021

Context

As Delphyne Guide states, the visualizer used to use the Plot plugin that was provided by ign-gui0.

During the migration to ign-gui3, it seems that Plot migration was omitted.

  • ign-gui3 doesn't provide a plot plugin
  • ign-gazebo3 provides a (Plot3D) plugin but it isn't meant to plot graphics but lines into the scene.
  • a plotting tool was under the scope of ignition folks: See this issue but now the issue is closed. This plotting tool is expected to match with Topic Viewer plugin so as to drag and drop variables to plot that are found with the Topic viewer widget.

EDITED:

  • The plotting tool didn't target citadel (ign-gui3) but it is implemented on Dome (ign-gui4).

Probably we could try to backport it to ign-gui3 or just add it here in delphyne_gui

Victory condition

Adds Plot plugin.

@francocipollone francocipollone changed the title Implement Plot plugin Plot plugin Sep 22, 2021
@francocipollone
Copy link
Collaborator Author

Probably we could try to backport it to ign-gui3 or just add it here in delphyne_gui

See gazebosim/gz-gui#289

I tried it locally backporting this plotting feature to ign-gui3 and it worked correctly with delphyne_gui:
Screenshot from 2021-09-23 12-06-08

@francocipollone
Copy link
Collaborator Author

It can't be ported because it breaks ABI.

We will need to migrate from citadel to a newer release.

Probably migrating to Fortress is the best option given that it is LTS, and it is being released at the end of September.
I honestly don't expect major changes. We could do a checkpoint at Edifice just to evaluate the impact.

@francocipollone
Copy link
Collaborator Author

I created a maliput worskpace from scratch using Edifice instead of Citadel on Delphyne and Delphyne-Gui and no changes were needed, visualizer is working as before but in addition, it has the Plotting feature

Here are the branches:

I think we can move to Edifice as it is effortless and later on migrate to Fortress. WDYT? @agalbachicar

@agalbachicar
Copy link
Collaborator

Great findings @francocipollone !

I think we can just wait one more week to get ign-fortress released ( see gazebo-tooling/release-tools#459 ) and then try it directly with the next LTS. Having checked that with ig-edifice it's just a change of version increases the probability that it will be the same with fortress. That way, we do the work just once.

@francocipollone
Copy link
Collaborator Author

Given that the final decision was not to migrate to a newer release and stay with citadel, I made an attempt to bring the Plotting feature. Some comments:

  • I brought ignition::gui::Application so as I could modified and add the change that breaks ABI in citadel.
  • After brining Application I had to bring other files/entities, like MainWindow, Plugin given that they inherited from QObject and I hadn't had access to the generated .hh files that QT generates with them.
  • Even though I was able to execute the visualizer there is a substantial issue:
    • The ign-gui3 builtin plugins won't work as expected(If work). Because they already depends on ignition::gui::Plugin and some of thems from ignition/gui/Application.hh file, which provides a method App() that returns a Application instance. So when this builtin plugins are used and they try to get the instance they point to an ignition::gui::Application (which doesn't exist) instead of to a delphyne::gui::Application, which causes a SEGFAULT.

This was worth trying but I don't see it valid to continue with this path as it compromises the built-in plugin on which we rely, unless we bring all the builtin plugins to delphyne-gui but It sounds a bit cumbersome.

@agalbachicar
Copy link
Collaborator

I think we can consider building an application from scratch that does not rely on the plugin architecture but implements the same plotting functionality offered by newer versions of the plugin.

We could think of coupling the application and the topic viewer via an ignition topic and service. Management of the plotting tool life cycle could be or not managed by the plugin. It changes (for worse) the user experience because of having another application instead of just one but could be a way to deal with the error for the time being.

@stonier
Copy link
Collaborator

stonier commented Oct 19, 2021

How about subprocess, not topics and services (that's getting complicated)?

@agalbachicar
Copy link
Collaborator

I think we missed to answer here, sorry about that. If you referred to use pipes for message transport between subprocesses I think we would do require message serialization and some kind of event system to gather data and visualize it. We could certainly do it with pipes for example but having ign-transport there which offers everything we need is tempting.

How relevant do you think this plugin is?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants