NASA Challenge_[@elementrobotics]_[LunarSim] (closes #50) #51
Conversation
mazm0002
changed the title
ivanperez-keera
left a comment
ivanperez-keera
left a comment
There was a problem hiding this comment.
This PR puts a lot of data in the demos repo (156000 lines more according to Github, and several megabytes worth of data). I believe some of that would belong in the https://github.com/space-ros/simulation/ repo, since it's models and such.
Can you please take a look at that repo, see how other demos are structured, see what would make sense to store there from your submission, send a PR to that repo, and adjust this one accordingly?
Thanks!
|
Hi @mazm0002 Just following up on this. |
|
Hi @ivanperez-keera, apologies for the delay. I've moved the world assets to the simulation repo and made a PR there space-ros/simulation#39. I'm pulling the assets into the demo docker by adding the fork of the simulation repo to the demo_manual_pkgs.repos file. This means that once the assets have been merged, the demo_manual_pkgs.repos file needs to be updated in this PR to point to the upstream simulation repo. Let me know if there's a better way to go about this. |
|
Thanks, @mazm0002 . Is there a way to make the screenshot image smaller? It's more than 1MB right now. Since it's meant to be read in the README, I'm hoping it'd be possible to make it smaller in size and compressing it a bit (perhaps making it a JPG?) without visible loss of precision. |
mazm0002
force-pushed
the
feature/lunarsim
branch
from
4ff3d8f to
ae56b51
Compare
|
@ivanperez-keera I've converted the image to a JPG and it's down to 426 kb now. |
|
@mkhansenbot @ivanperez-keera Any idea why this is failing in CI now with this error? |
looks like all CIs are failing due to space issues |
|
Is this based on the LunarSim project out of PUT, or a different thing? I don't want any confusion between the (possibly many) different things called "LunarSim" in our demos |
This demo isn't associated with the one from PUT, but I wouldn't mind changing the name to avoid any confusion with other demos. Open to any suggestions, maybe something like "lunar_rover" or "lunar_terrain"? |
|
either of those names sound good to me! |
|
I like |
|
I vote for the main value is the Gazebo lunar environment with the sun plugin IMHO |
|
@mazm0002 Are you ok with that change? I'd like to help get this PR across the finish line. Could you please give me write access to your fork? I can confirm that these changes rebase cleanly on top of There's a small amount of re-org that we'll want to do in the history. There's also a small change we need to make in the Dockerfile (it's failing to build atm due to a GPG key). I fixed that on my end and I'm building this image. Still going. |
|
This is the next error I hit: |
|
Hi @ivanperez-keera, sorry for the delays with this, I've been preoccupied with other work. I've given you write access to the repo, thanks for assisting in getting this merged. I can try reproduce the issue you're running into this weekend and see if I can fix it. And yes definitely fine with the name change. |
|
@mazm0002 let me know if you are able to reproduce the issue. I'm still finding that on my end. Thanks! |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces LunarSim, a comprehensive Gazebo Garden-based lunar simulation for SpaceROS that provides a realistic lunar environment around the Shackleton Crater using NASA's Digital Elevation Model (DEM) data and dynamic sun modeling based on ephemeris data.
Key changes include:
- Implementation of a lunar world simulation with accurate terrain modeling from LOLA DEM data
- Dynamic sun positioning system using NASA Horizons ephemeris data with a custom Gazebo plugin
- Surface texture enhancement through normal map blending and processing tools
Reviewed Changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| Dockerfile | Main container configuration building on SpaceROS base with Gazebo Garden |
| run.sh | Docker container execution script with GUI passthrough |
| lunar_sun_gz_plugin/ | Gazebo plugin for dynamic sun movement based on ephemeris data |
| lunarsim_gz_worlds/ | World files, launch scripts, and DEM processing tools |
| lunarsim_gz_bringup/ | Launch configurations for spawning Leo rover with teleoperation |
| build.sh | Build script for creating the demo Docker image |
Comments suppressed due to low confidence (1)
lunarsim_demo/lunar_sun_gz_plugin/CHANGELOG.rst:2
- The changelog header refers to 'leo_gz_plugins' but this is actually the changelog for 'lunar_sun_gz_plugin'. The package name should be corrected.
Changelog for package leo_gz_plugins
lunarsim_demo/lunarsim_gz_bringup/launch/lunarsim_world.launch.py
Outdated
Show resolved
Hide resolved
|
@ivanperez-keera it doesn't seem like Copilot picked up on what might be wrong in the CMake (at least in this first pass) but it did seem to generate some other helpful review comments. |
|
I'm running the build, however I don't expect it to pass until #105 is merged (hopefully soon). I'm pushing to get the builds passing again, once we do, I'll re-run this one and help get it merged |
|
@mkhansenbot how are these related? I have a commit in my work machine when I built everything completely. I run into issues with keys and probably did something similar to what you did in the ROS trick demo, if that's what you are talking about. |
|
@ivanperez-keera - they're not directly related, but the |
mazm0002
force-pushed
the
feature/lunarsim
branch
from
43ce62f to
9e6261c
Compare
|
Build failed since the GPG key fix for the other demos hadn't been included. I've rebased the branch, builds fine on my local now, could you rerun the build @mkhansenbot? 🤞 |
|
Just an fyi, space-ros/simulation#39 should be merged in before this one, and then this PR needs to be updated to point to the main repo instead of our fork in lunar_terrain/demo_manual_pkgs.repos |
|
This is passing CI, I think it just needs final approval from @ivanperez-keera |
ivanperez-keera
force-pushed
the
feature/lunarsim
branch
from
9e6261c to
f1ae057
Compare
|
We are able to run the example but we are seeing error in the terminal that seem related to missing files or resources? This is the image we see as we move around. Does this look right to you? 
|
|
For what is worth, I had to make these changes to make things work on my machine. These may not work on all machines: $ git diff
diff --git a/lunar_terrain/run.sh b/lunar_terrain/run.sh
index ae941ae..a580b1a 100755
--- a/lunar_terrain/run.sh
+++ b/lunar_terrain/run.sh
@@ -12,5 +12,6 @@ IMG_NAME=openrobotics/space_robots_lunar_terrain
CONTAINER_NAME="$(tr '/' '_' <<< "$IMG_NAME")"
# Start the container
-docker run --rm -it --name "$CONTAINER_NAME" --network host \
+docker run -v /dev/dri:/dev/dri --rm -it --name "$CONTAINER_NAME" --network host \
+ -v /tmp/.X11-unix:/tmp/.X11-unix \ |
ivanperez-keera
left a comment
ivanperez-keera
left a comment
There was a problem hiding this comment.
The lunar_sun_gz_plugin contains a CHANGELOG that seems to refer to a different project. Can you explain?
ivanperez-keera
force-pushed
the
feature/lunarsim
branch
from
f1ae057 to
f0a6baf
Compare
|
We evaluated this and it seems good to merge except for the question about the changelog indicated above: Does the changelog, which seems to refer to a different project, need to be kept. If so, does anything in it need to be changed. Does anybody need to be credited (e.g., if your starting point was a different package that you used as template and modified). |
|
Tagging @mazm0002 for awareness. |
ivanperez-keera
force-pushed
the
feature/lunarsim
branch
3 times, most recently
from
3fff5d9 to
bb1949c
Compare
@ivanperez-keera IIRC we did use some code from the leo_simulator library as a template, but have kept their (Fictionlab) copyright license notice on all the files that were derived from theirs. The changelog doesn't need to be kept, I will remove it. |
mazm0002
force-pushed
the
feature/lunarsim
branch
from
bb1949c to
10ec564
Compare
ivanperez-keera
force-pushed
the
feature/lunarsim
branch
from
10ec564 to
e736268
Compare
|
FYI: looks like simulation#39 was merged |
|
Before merging this, we need to adjust the URL to the simulator repo. It was pointing to their branch. That commit should be squashed with the current commit. |
Add a new demo that includes a rover moving around the moon. The demo features lunar terrain and lighting. This demo depends on assets included in the simulation repository. Co-authored-by: Munir Azme <[email protected]>
mazm0002
force-pushed
the
feature/lunarsim
branch
from
e736268 to
fa0967b
Compare
@ivanperez-keera I've just updated the URL and squashed the commit. |
7 participants
LunarSim
Resolves #50, depends on space-ros/docs#36 and space-ros/simulation#39
This PR adds a Gazebo Garden based lunar world demo to spaceROS. The features included in this PR are:
This package serves as a foundation for creating a realistic lunar simulation that the community can further expand upon. To support this, we have provided comprehensive documentation that guides users through the process of preparing and adding their own DEMs, ephemeris data, and custom textures. These tutorials can be found in our space-ros/docs PR ( space-ros/docs#36)
Instructions on how to run the demo can be found in the README. This image builds on top of the spaceROS base image and follows the format used in other demos for consistency.