-
Notifications
You must be signed in to change notification settings - Fork 9
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
Creating a geometrical model #192
Conversation
…miliar. Also pushig to test the branch on the fork.
…o get familiar. Also pushig to test the branch on the fork." This reverts commit 9d8482d.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some comments / suggestions
biggest question to settle for me atm is whether we support meshes at all. if so, we should structure the code around it. However if the math is too tricky with generic meshes, then maybe we just stick to cuboid everywhere. what do you think @GabrieleMeoni ?
paseos/attitude/geometric_model.py
Outdated
Returns: | ||
np.array: Coordinates of the center of gravity of the mesh | ||
""" | ||
return np.array([0, 0, 0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably not correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on how the object was made, this was correct (default was to center the satellite around the origin of the satellite reference frame). But with the new mesh method proposed in the issue, this will be changed
Co-authored-by: Pablo Gómez <[email protected]>
Co-authored-by: Pablo Gómez <[email protected]>
…project # Conflicts: # paseos/actors/actor_builder.py
@Moanalengkeek please request review with the arrow buttons under review when I should have another look ;) PS. also added you to the repo so the CI runs automatically (before I had to manually start the tests so it makes sense adding you after all ;) ) |
Thanks! Will do. I also tied to find where the tests failed, but it seems to be in a place I didn't change. Would you have any resources for best troubleshooting of such tests? |
Hi @Moanalengkeek and everyone. First of all, thanks to @gomezzz for the support in providing an exact revision. Thanks to you for your contribution to PASEOS. I think your implementation generally looks good. Supporting a mesh is for long-term vision, but I would stick to a Cuboid, as Pablo suggested, for the next steps. Before approving this PR, please improve the comments in the code. A few things shall be clarified:
In terms of reference frame, in the frame of the next PR you should create a new directory inside PASEOS, providing utilities to transform your frame to one attached to Earth, which is needed for the pointing vector coordinates. |
Moved to #195, which does not need a fork. |
Description
Adding a geometric model to the model. This is done with two possibilities; option 1 is to model a basic cuboid with width, length, and height. Option 2 is to import an .obj file, and to use the mesh from that.
Summary of changes
Resolved Issues
How Has This Been Tested?
Tested by the two tests mentioned added in actor_builder_test.py, as well as running a simple code that generates an actor and creates the geometry
Related Pull Requests