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

Code simplification, kinematic loop example #5

Closed
wants to merge 0 commits into from

Conversation

rohit-kumar-j
Copy link
Contributor

@rohit-kumar-j rohit-kumar-j commented May 14, 2022

Added a Kinematic Loop example
Simplified the mujoco_viewer.py with callbacks in another file.
Auto creates a root/tmp (unless root/tmp exists) to save screen captures in it.

TODO: No.2 in #4

@rohit-kumar-j rohit-kumar-j marked this pull request as ready for review May 14, 2022 06:50
@rohanpsingh
Copy link
Owner

Hi @rohit-kumar-j
Thanks for the PR.

I have several comments and some questions.

  • Why do you want to put the callbacks in another file? The code is pretty straightforward as-is and easy to follow. Unless the callbacks are needed to be used in another place, I'm not sure if breaking the file into 2 has any use.
  • It might be better to save the screencap frames under $HOME/.mujoco-viewer, so a new directory is not created each time the user changes to a new os.getcwd()
  • The Kinematic Loop example may be useful for other users too. Can you create a separate PR for the example?

@rohit-kumar-j
Copy link
Contributor Author

Why do you want to put the callbacks in another file? The code is pretty straightforward as-is and easy to follow. Unless the callbacks are needed to be used in another place, I'm not sure if breaking the file into 2 has any use.

I was adding graphs, and thought it would be easier to restructure the code to reduce the cluttering up a single file

It might be better to save the screencap frames under $HOME/.mujoco-viewer, so a new directory is not created each time the user changes to a new os.getcwd()

This is a better idea. will do.

The Kinematic Loop example may be useful for other users too. Can you create a separate PR for the example?

Certainly

@rohit-kumar-j
Copy link
Contributor Author

Graphing example in examples/graphs.py in a dev branch. not ready or pr yet.

Preview:
The sinewave is a custom data_stream with math.sin(x)
Rest of them are sensor data

mujoco_graph_working.mp4

@rohanpsingh
Copy link
Owner

Looks good, I will take a look at your branch in a few days!

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

Successfully merging this pull request may close these issues.

None yet

2 participants