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

Added log storing for ign-gui #272

Merged
merged 3 commits into from
Nov 11, 2021
Merged

Added log storing for ign-gui #272

merged 3 commits into from
Nov 11, 2021

Conversation

NickNair
Copy link
Contributor

@NickNair NickNair commented Aug 15, 2021

🎉 New feature

Closes #962 from ign-gazebo repository

Summary

Every time a simulation is run with the GUI, the GUI console logs are saved to ~/.ignition/gazebo/log//gui_console.log

Test it

Run a simulation. Then :
`cd /.ignition/gazebo/log/

cat gui_console.log`

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

@github-actions github-actions bot added the 🏢 edifice Ignition Edifice label Aug 15, 2021
@NickNair
Copy link
Contributor Author

How do I write a test for this PR?

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! I just have a small comment. Nice idea to put this into ignitionVersion, since that's called every time the user runs ign gui!

src/ign.cc Outdated Show resolved Hide resolved
@chapulina
Copy link
Contributor

How do I write a test for this PR?

You could write a test similar to this one:

https:/ignitionrobotics/ign-gui/blob/6cdc2f7021e9435d62560f68c273ffad2f6a19d3/src/ign_TEST.cc#L56-L61

And check that the file was created.

I recommend setting the $HOME environment variable to a fake path like this:

https:/ignitionrobotics/ign-gazebo/blob/7d1a3e09dc596a4bbee968a955949b55b08629cd/test/helpers/EnvTestFixture.hh#L43-L44

So that when the user runs tests, no files are created in their home folder.

@chapulina chapulina changed the base branch from ign-gui5 to ign-gui3 November 8, 2021 22:54
@chapulina chapulina added 🏰 citadel Ignition Citadel and removed 🏢 edifice Ignition Edifice labels Nov 8, 2021
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Thanks again for the PR, @NickNair !

I rebased on top of ign-gui3 (Citadel), and after merged we can merge it forward.

Also pushed these changes in b9d8345:

  • Changed directory from gazebo to gui
  • Moved functionality to a new function for clarity, so in case we stop calling ignitionVersion every time, this still works
  • Added a test
  • Added a note to the tutorial

Maybe @jennuine can take a look too since I've touched a lot of the PR?

Copy link
Contributor

@jennuine jennuine left a comment

Choose a reason for hiding this comment

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

LGTM, one minor comment

src/ign_TEST.cc Outdated Show resolved Hide resolved
@chapulina chapulina merged commit 970914a into gazebosim:ign-gui3 Nov 11, 2021
chapulina added a commit that referenced this pull request Dec 8, 2021
* Added log storing for ign-gui (#272)

Signed-off-by: Nikhil Nair <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>

Co-authored-by: Louise Poubel <[email protected]>

* Don't crash if a plugin has invalid QML (#315)

Signed-off-by: Louise Poubel <[email protected]>

* Set marker point size from message (#317)

Signed-off-by: Louise Poubel <[email protected]>

* Don't set visual scale for point markers (#321)

Signed-off-by: Louise Poubel <[email protected]>

* Fix TopicEcho plugin message display (#322)

- Change binding of width property in delegate (see: https://stackoverflow.com/questions/63767669/parent-is-null-in-listview-delegate-after-upgrade-to-qt-5-15)
- Use scoped reference to model.display (see: https://forum.qt.io/topic/92085/using-qstringlistmodel-as-model-in-listview)

Signed-off-by: Rhys Mainwaring <[email protected]>

* Use qmldir to define QML module with IgnSpinBox (#319)

Signed-off-by: William Wedler <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>

* Add PreRender event to MinimalScene (#325)

Signed-off-by: Louise Poubel <[email protected]>

* Offer a way to disable warnings on marker manager (#326)

Signed-off-by: Louise Poubel <[email protected]>

Co-authored-by: Alejandro Hernández Cordero <[email protected]>

* Fix codecheck (#329)

Signed-off-by: Louise Poubel <[email protected]>

* Fix codecheck (#332)

Signed-off-by: Louise Poubel <[email protected]>

* Grid config: set values from startup and improve layout (#324)

Signed-off-by: Louise Poubel <[email protected]>

Co-authored-by: Nikhil Nair <[email protected]>
Co-authored-by: Rhys Mainwaring <[email protected]>
Co-authored-by: Will <[email protected]>
Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Co-authored-by: Jenn Nguyen <[email protected]>
@osrf-triage
Copy link

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-01-10/1228/1

@osrf-triage
Copy link

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-01-24-citadel-edifice-fortress/1241/1

@osrf-triage
Copy link

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-03-01-citadel-edifice-fortress/1313/1

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

Successfully merging this pull request may close these issues.

Save GUI console logs
4 participants