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

Creating a geometrical model #192

Closed
wants to merge 17 commits into from

Conversation

Moanalengkeek
Copy link
Collaborator

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

  • added function set_cuboid_geometric_model to the file actor_builder.py
  • added function set_geometric_model_from_import to the file actor_builder.py
  • added folder attitude
  • added file geometric_model.py to attitude folder
  • added class CuboidGeometricModel to geometric_model.py
  • added class ImportGeometricModel to geometric_model.py
  • added function test_set_geometric_model to actor_builder_test.py
  • added function test_view_geometric_model to actor_builder_test.py

Resolved Issues

  • fixes 191

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

  • None

Copy link
Collaborator

@gomezzz gomezzz left a 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 ?

objects/20265_Hexagonal_prism_v1.obj Outdated Show resolved Hide resolved
objects/Blank image.jpg Outdated Show resolved Hide resolved
paseos/actors/actor_builder.py Outdated Show resolved Hide resolved
paseos/actors/actor_builder.py Outdated Show resolved Hide resolved
paseos/actors/actor_builder.py Outdated Show resolved Hide resolved
paseos/attitude/geometric_model.py Outdated Show resolved Hide resolved
Returns:
np.array: Coordinates of the center of gravity of the mesh
"""
return np.array([0, 0, 0])
Copy link
Collaborator

Choose a reason for hiding this comment

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

probably not correct?

Copy link
Collaborator Author

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

paseos/attitude/geometric_model.py Outdated Show resolved Hide resolved
paseos/tests/actor_builder_test.py Outdated Show resolved Hide resolved
paseos/tests/actor_builder_test.py Outdated Show resolved Hide resolved
@gomezzz
Copy link
Collaborator

gomezzz commented Jan 5, 2024

@Moanalengkeek please request review with the arrow buttons under review when I should have another look ;)

image

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 ;) )

@Moanalengkeek
Copy link
Collaborator Author

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?

@Moanalengkeek Moanalengkeek requested a review from gomezzz January 8, 2024 10:09
@GabrieleMeoni
Copy link
Collaborator

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:

  • What vertices and faces are and how they are linked is not clear without accessing Trimesh documentation. Please briefly describe how to define faces w.r.t. vertices in the function description.
  • What unit measurements are you using for vertices and faces?
  • What reference frame are you using? It seems to be a reference frame attached to the spacecraft. Is this correct? Please consider this good for mesh and inertia calculation, but you would need to comply with the PASEOS general reference frame, which is attached to Earth.

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.

paseos/actors/actor_builder.py Outdated Show resolved Hide resolved
paseos/actors/actor_builder.py Outdated Show resolved Hide resolved
paseos/attitude/geometric_model.py Outdated Show resolved Hide resolved
paseos/attitude/geometric_model.py Outdated Show resolved Hide resolved
@GabrieleMeoni
Copy link
Collaborator

Moved to #195, which does not need a fork.

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.

4 participants