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

Cleanup planning scene geometry code #37

Merged
merged 3 commits into from
Aug 25, 2024
Merged

Conversation

sea-bass
Copy link
Contributor

@sea-bass sea-bass commented Aug 24, 2024

This PR cleans up some of the repetitive code in the planning scene geometry transcription, plus some other small stuff here and there.

I could get the code to compile, but for some reason the vcs import part of the container build is failing... so I'm unable to test it out on the latest image build. Can you guys try this out @sjahr @kamiradi?

I get this error:

Could not clone repository 'https://github.com/sjahr/moveit_benchmark_resources':
fatal: destination path '.' already exists and is not an empty directory.

@sea-bass
Copy link
Contributor Author

sea-bass commented Aug 24, 2024

Maybe this is the issue. When I manually try to clone your fork of the repo @sjahr , I get this error:

Downloading moveit_benchmark_resources/databases/panda_benchmarks.sqlite (4.3 MB)
Error downloading object: moveit_benchmark_resources/databases/panda_benchmarks.sqlite (43e9f02): 
Smudge error: Error downloading moveit_benchmark_resources/databases/panda_benchmarks.sqlite (43e9f02577ef1eda71d0ac2e63c79522791f78c3ef69fc24814af3def90bf299):
batch response: This repository is over its data quota. 
Account responsible for LFS bandwidth should purchase more data packs to restore access.

Seems to also happen on the upstream moveit branch -- the perils of git-lfs

@sjahr
Copy link
Contributor

sjahr commented Aug 25, 2024

Oh no ... How can we fix this? Do you know if it is bandwidth / month and it will resolve itself next month?

@sjahr
Copy link
Contributor

sjahr commented Aug 25, 2024

I looked a bit into it and we might have a problem if it does not reset on a monthly basis because otherwise we would need to lfs storage. A short-term workaround could also be to add the test database to this repository

Copy link
Contributor

@sjahr sjahr left a comment

Choose a reason for hiding this comment

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

Tested it and it worked once you've included my comment (otherwise I get a compile error)

@sea-bass
Copy link
Contributor Author

According to https://docs.github.com/en/repositories/working-with-files/managing-large-files/about-storage-and-bandwidth-usage, the overall storage caps out at 1GB, but bandwidth is 1GB per month. So it does reset.

But yeah, on another personal project I used to have a few GIFs on LFS and in a period of high activity it exceeded my quota. So I just brought it out to regular storage.

How big are these databases? If it's on the order of 10s of MBs, might be worth doing the same here. But if it's 100s... not sure. Maybe providing an external download link?

@sea-bass sea-bass merged commit d6bfb28 into main Aug 25, 2024
2 checks passed
@sea-bass sea-bass deleted the cleanup-scene-geometry-parsing branch August 25, 2024 14:32
Copy link
Member

@kamiradi kamiradi left a comment

Choose a reason for hiding this comment

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

A bit too late but thumbs up from me :)

// dome drake related scope initialisations
const auto& plant = dynamic_cast<const MultibodyPlant<double>&>(diagram_->GetSubsystemByName("plant"));
// some drake related scope initialisations
const auto& plant = diagram_->GetDowncastSubsystemByName<MultibodyPlant<double>>("plant");
Copy link
Member

Choose a reason for hiding this comment

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

Ah nice, I knew there was a better way to do this 🫡

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.

3 participants