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

moved physics tutorials to examples under the heading 'Integrate physics using pybullet' #302

Merged
merged 3 commits into from Aug 25, 2020
Merged

Conversation

tushar5526
Copy link
Contributor

@tushar5526 tushar5526 commented Aug 23, 2020

#297
Moved all the Physics tutorials under examples section with a heading 'Integrate physics using pybullet'

Fixed the buggy wrecking ball behavior, the problem was radii was the links or rope's radius instead of the wrecking ball. Changing it to 1 was making a cylindrical collider of radius 1 greater than wrecking ball's radius hence the bug.
Ball's radius was hardcoded to 0.2, assigned it to the variable ball_radius so now you can mess with wrecking ball by changing the radius.
Fixed some typos that I found along with it.

PS : There was some weird behavior after 100 simulation steps. The ball went crazy and there were some weird stretched lines instead of ropes, I think the code to make the rope has to be redone.

@tushar5526 tushar5526 changed the title moved physics tutorials to examples with heading 'Integrate physics using pybullet' moved physics tutorials to examples under the heading 'Integrate physics using pybullet' Aug 23, 2020
@skoudoro skoudoro added this to the v0.7.0 milestone Aug 23, 2020
@Nibba2018
Copy link
Member

Hello @tushar5526 , Thank you for creating this PR. As you are currently working on the physics examples, I would request you to work on this problem so that we can close #298 as well.

@skoudoro
Copy link
Contributor

skoudoro commented Aug 23, 2020

Hi @tushar5526,

Thank you for doing this!

+1 with @Nibba2018 request
Furthermore, can you add pybullet in the requirements/docs.txt?

Thank you

@tushar5526
Copy link
Contributor Author

Sure @Nibba2018 @skoudoro :)

@Nibba2018
Copy link
Member

Nibba2018 commented Aug 23, 2020

Also a quick note regarding the installation of pybullet. Most windows versions may or may not have the appropriate wheels for pybullet. Therefore, pip install pybullet will throw an error, and in order to fix that one must install Windows SDK using Visual Studio Build Tools from here.

The same problem doesn't occur for Linux or mac os. So no issues with that.

@tushar5526
Copy link
Contributor Author

@Nibba2018 Thanks, I work on Ubuntu 18.04 LTS so no issues

@Nibba2018
Copy link
Member

Nibba2018 commented Aug 23, 2020

Which section do you think is appropriate to add this information @skoudoro ?

@tushar5526
Copy link
Contributor Author

Any version barrier with pybullet @Nibba2018 ?

@Nibba2018
Copy link
Member

That's a very hard question to answer as I was never able to find any changelogs for pybullet. But anything above 2.7.0 should work fine.

@skoudoro
Copy link
Contributor

Which section do you think is appropriate to add this information @skoudoro ?

Many packages have this issue when they have a C or cython dependencies. I think it is ok. No need to add this information until we decide that is a mandatory dependency.

@codecov
Copy link

codecov bot commented Aug 23, 2020

Codecov Report

Merging #302 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #302   +/-   ##
=======================================
  Coverage   88.64%   88.64%           
=======================================
  Files          21       21           
  Lines        5073     5073           
  Branches      656      656           
=======================================
  Hits         4497     4497           
  Misses        406      406           
  Partials      170      170           

@tushar5526
Copy link
Contributor Author

@Nibba2018 No output on running viz_wrecking_ball.py. When I am running it a screen flashes for 0.5 seconds and then the program exits, plus a png file is created that is completely black. No errors or warnings in the console.
pybullet==2.8.7 ( latest version )

@Nibba2018
Copy link
Member

@tushar5526 Have you set the value of interactive to True? Also, you need to increase the simulation step limit(if cnt == 2000) to view the entire simulation. The limit was purposefully set to a lower value for doc generation.

@tushar5526
Copy link
Contributor Author

Thanks @Nibba2018 works good !

@pep8speaks
Copy link

pep8speaks commented Aug 23, 2020

Hello @tushar5526! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 39:80: E501 line too long (80 > 79 characters)

Line 95:80: E501 line too long (86 > 79 characters)
Line 118:80: E501 line too long (80 > 79 characters)

Comment last updated at 2020-08-25 04:26:15 UTC

@tushar5526
Copy link
Contributor Author

tushar5526 commented Aug 23, 2020

Fixed the buggy wrecking ball behavior, the problem was radii was the links or rope's radius instead of the wrecking ball. Changing it to 1 was making a cylindrical collider of radius 1 greater than wrecking ball's radius hence the bug.
Ball's radius was hardcoded to 0.2, assigned it to the variable ball_radius so now you can mess with wrecking ball by changing the radius.
Fixed some typos that I found along with it.

PS : There was some weird behaviour after 100 simulation steps. The ball went crazy and I was having weird stretched lines instead of ropes, I think the code to make the rope has to be redone.

@Nibba2018 @skoudoro

@skoudoro
Copy link
Contributor

Great job @tushar5526! do not forget to address the pep8 above

@tushar5526
Copy link
Contributor Author

tushar5526 commented Aug 23, 2020

Do you want the commits to be squashed or is it ok ? @skoudoro

@skoudoro
Copy link
Contributor

No need to squash

Copy link
Contributor

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

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

Hi @tushar5526,

Your PR is almost ready to go. I just generate the documentation and see below the last issue.
After your fixes, we can merge it.

Thank you for this last update

docs/examples/physics_using_pybullet/README.rst Outdated Show resolved Hide resolved
docs/examples/physics_using_pybullet/viz_wrecking_ball.py Outdated Show resolved Hide resolved
docs/examples/physics_using_pybullet/viz_wrecking_ball.py Outdated Show resolved Hide resolved
Increasing 'radii' was changing the link's radius instead of the wrecking ball.
Fixed some typos in Physics simulation folder.

fixed physics demos according to pep8
@tushar5526
Copy link
Contributor Author

Thanks, @skoudoro for checking it out, made the changes, and also ran pep8 on other physics demos files.

@skoudoro skoudoro linked an issue Aug 25, 2020 that may be closed by this pull request
@skoudoro skoudoro merged commit 5692267 into fury-gl:master Aug 25, 2020
@skoudoro
Copy link
Contributor

Thank you @tushar5526. Merging

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

Successfully merging this pull request may close these issues.

Wrecking Ball Simulation
4 participants