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

Notebook urban dyn #364

Merged
merged 15 commits into from
Aug 4, 2023
Merged

Notebook urban dyn #364

merged 15 commits into from
Aug 4, 2023

Conversation

han16nah
Copy link
Contributor

@han16nah han16nah commented Aug 2, 2023

New notebook for demonstrating rigid motions: MLS simulation in an urban environment with two moving vehicles

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Contributor

@lwiniwar lwiniwar left a comment

Choose a reason for hiding this comment

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

looks good, nice showcase of the dynamic scenes capabilities!
I added some minor changes/ideas to consider

Copy link
Contributor

Choose a reason for hiding this comment

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

can we remove the commented position noise here to keep it a bit more readable?

Ka 0.000000 0.000000 0.000000
Kd 0.700000 0.700000 0.700000
Ks 0.700000 0.700000 0.700000
helios_classification 3
Copy link
Contributor

Choose a reason for hiding this comment

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

can we set classification of building to "6" as per the ASPRS Standard?

# www.blender.org

newmtl vegetation
helios_classification 2
Copy link
Contributor

Choose a reason for hiding this comment

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

vegetation should be 3, 4 or 5 (low, med, high)

# www.blender.org

newmtl vegetation
helios_classification 2
Copy link
Contributor

Choose a reason for hiding this comment

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

I would give class 5 here

@lwiniwar
Copy link
Contributor

lwiniwar commented Aug 2, 2023

Also, what is the license on the .jpg textures - and do we really need them? They don't make a difference for HELIOS, so I would say to remove them.

@han16nah han16nah requested a review from lwiniwar August 3, 2023 10:39
Copy link
Contributor

@lwiniwar lwiniwar left a comment

Choose a reason for hiding this comment

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

nice job, lgtm!

scanAngleEffectiveMax_deg = "1"
scanFreqMin_Hz = "0"
scanFreqMax_Hz = "0"
wavelength_nm = "905"
Copy link
Contributor

Choose a reason for hiding this comment

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

also 903 nm according to https://www.goetting-agv.com/dateien/downloads/63-9194_Rev-G_HDL-64E_S3_Spec%20Sheet_Web.pdf

.. not that it makes a difference, but the correction for the VLP-16 made me wonder.

@han16nah han16nah merged commit afadced into dev Aug 4, 2023
17 checks passed
@han16nah han16nah deleted the notebook_urban_dyn branch August 23, 2023 09:47
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.

2 participants