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

ENH: Sensors #583

Merged
merged 83 commits into from
May 21, 2024
Merged

ENH: Sensors #583

merged 83 commits into from
May 21, 2024

Conversation

MateusStano
Copy link
Member

@MateusStano MateusStano commented Apr 2, 2024

Pull request type

  • Code changes (bugfix, features)

Checklist

  • Tests for the changes have been added (if needed)
  • Docs have been reviewed and added / updated
  • Lint (black rocketpy/ tests/) has passed locally
  • All tests (pytest tests -m slow --runslow) have passed locally
  • CHANGELOG.md has been updated (if relevant)

Description

This PR tackles issue #273 and builds on top of #580

Added:

  • Sensors abstract class
  • Accelerometer class
  • Gyroscope class
  • Sensors are included in the Flight simulation loop. Having any amount of sensors in the simulated rocket will add only one additional call to u_dot per simulation step. Sensors only work time_overshoot=False
  • add_sensor method in rocket class that receives the sensor and its position and saves them in a Component object
  • __and__ operator for elementwise multiplication between Vector objects
  • euler_to_quaternions function in tools.py for conversion from euler angles to quaternions
  • transformation_euler_angles static method in the Matrix class for construction of a rotation matrix from euler angles
  • Sensors representation in the rocket drawing by their position and normal vector, given a choice of plane for the drawing
  • Small refactor of Components class for speed purposes
  • Parachute triggerfunc can now receive a list of sensors as its last argument
  • Controller controller_function can now receive a list of sensors as its last argument

Sensors Architecture

  • Sensors have their orientation and noise information saved in itself. When added to the rocket, a position must be given
  • Each type of sensor has its own measure method that characterizes the sensor. The measure function receives (t, state, state_derivative, *args). *args refers to the specific input required by the sensor. Different types of sensors will receive different *args.
  • At each call of the measure method, the sensor's reading is saved in the measured_data attribute. This means that the state of the sensor changes throughout the simulation.

Notes

  • I created a notebook called sensors_testing.ipynb for testing the current sensor implementation. It does not need to be reviewed
  • The modulus of all axes of an accelerometer/gyroscope without noise placed in the rocket's center of dry mass should be the same as the flight's acceleration/rotation modulus. This is tested in test_ideal_sensors. The tolerance in this test is limited by numerical errors in the calculation of the rotation matrix from the quaternions. This is probably causing inaccuracies in the simulation. We should fix this soon
  • At time 3.9s there is a dissonant acceleration value in the z axis of the Calisto flight. Seems to be related to errors in the derivation of the motor's mass_dot. This does not happen when the old udot is used. We should fix this soon
  • Useful links:

Breaking change

  • Yes
  • No

@MateusStano MateusStano requested a review from a team as a code owner April 2, 2024 16:53
Copy link

codecov bot commented Apr 2, 2024

Codecov Report

Attention: Patch coverage is 90.80460% with 48 lines in your changes are missing coverage. Please review.

Please upload report for BASE (enh/sensors-impl@fbeff2c). Learn more about missing BASE report.

Files Patch % Lines
rocketpy/plots/rocket_plots.py 30.43% 16 Missing ⚠️
rocketpy/sensors/sensors.py 87.35% 11 Missing ⚠️
rocketpy/simulation/flight.py 91.79% 11 Missing ⚠️
rocketpy/control/controller.py 75.00% 3 Missing ⚠️
rocketpy/mathutils/vector_matrix.py 90.90% 2 Missing ⚠️
rocketpy/prints/sensors_prints.py 98.14% 1 Missing ⚠️
rocketpy/rocket/components.py 85.71% 1 Missing ⚠️
rocketpy/rocket/rocket.py 90.00% 1 Missing ⚠️
rocketpy/sensors/accelerometer.py 98.48% 1 Missing ⚠️
rocketpy/sensors/gyroscope.py 98.55% 1 Missing ⚠️
Additional details and impacted files
@@                 Coverage Diff                 @@
##             enh/sensors-impl     #583   +/-   ##
===================================================
  Coverage                    ?   73.78%           
===================================================
  Files                       ?       64           
  Lines                       ?    10049           
  Branches                    ?        0           
===================================================
  Hits                        ?     7415           
  Misses                      ?     2634           
  Partials                    ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@phmbressan phmbressan left a comment

Choose a reason for hiding this comment

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

This is a great start for RocketPy Sensors feature. Well done @MateusStano and thank you for providing references in the PR description.

Before approval, I have made some comments, but overall I liked the implementation a lot.

rocketpy/mathutils/vector_matrix.py Show resolved Hide resolved
Comment on lines +192 to +194
plane : str, optional
Plane in which the rocket will be drawn. Default is 'xz'. Other
options is 'yz'. Used only for sensors representation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would detail a bit more where these axes are defined (e.g. the x axis is the one defined as reference in the sensor definition). Perhaps even link the docs page.

A future enhancement would be the correct angular position of the rail buttons, even though we don't have that information right now.

Copy link
Member

Choose a reason for hiding this comment

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

rocketpy/plots/rocket_plots.py Outdated Show resolved Hide resolved
rocketpy/prints/sensors_prints.py Show resolved Hide resolved
rocketpy/prints/sensors_prints.py Outdated Show resolved Hide resolved
rocketpy/sensors/gyroscope.py Outdated Show resolved Hide resolved
rocketpy/control/controller.py Show resolved Hide resolved
rocketpy/mathutils/vector_matrix.py Outdated Show resolved Hide resolved
rocketpy/sensors/gyroscope.py Outdated Show resolved Hide resolved
tests/unit/test_sensors.py Outdated Show resolved Hide resolved
@Gui-FernandesBR Gui-FernandesBR linked an issue Apr 4, 2024 that may be closed by this pull request
@Gui-FernandesBR Gui-FernandesBR added this to the Release v1.X.0 milestone Apr 4, 2024
@Gui-FernandesBR Gui-FernandesBR added Enhancement New feature or request, including adjustments in current codes Controllers Controlling rocket flight methods Flight Flight Class related features labels Apr 4, 2024
@phmbressan
Copy link
Collaborator

I forgot to comment in the review:

  • There must be a warning in the Flight class if a Rocket has AirBrakes and overshootable is True. The same apply for Controllers.

@Gui-FernandesBR
Copy link
Member

@MateusStano , tomorrow I will invest some time to test the jupyter notebook and search for any bug.
In the mean time, there are still a few conversations opened, would you mind addressing them?

This is a already too large PR (70 commits), feel free to write down some TODOs in the code if you think something is not crucial, just let us know please.

Btw the head branch seems to be outdated with respect to the develop branch.

Copy link
Member

@Gui-FernandesBR Gui-FernandesBR left a comment

Choose a reason for hiding this comment

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

Good work addressing most of the review comments. I know sometimes it can be hard, but please go through all the open conversations. You can let some TODO in the code if needed.

Let me know when you are done.

I have used the sensors notebook and found some inconsistent information. Can you take a look?
image
image
Seems like we have an inverted direction somewhere:
image

rocketpy/mathutils/vector_matrix.py Show resolved Hide resolved
rocketpy/prints/sensors_prints.py Outdated Show resolved Hide resolved
rocketpy/rocket/parachute.py Show resolved Hide resolved
rocketpy/rocket/rocket.py Outdated Show resolved Hide resolved
rocketpy/sensors/sensors.py Outdated Show resolved Hide resolved
rocketpy/sensors/sensors.py Show resolved Hide resolved
@MateusStano
Copy link
Member Author

Seems like we have an inverted direction somewhere:
image

This result is supposed to be right because the noisy gyroscope is upside down. Please check the orientation and see if it makes any sense

Copy link
Member

@Gui-FernandesBR Gui-FernandesBR left a comment

Choose a reason for hiding this comment

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

Approving this because:

  • I know you made a good job
  • This is not going to develop yet
  • We can make some small adjustments before merging to develop later.

I think 1 or 2 comments/suggestions were left behind, but hopefully we will double click on them in future PRs, no problem.

Also I did not care so much about the new jupyter notebook, I think it is there as a fast testing option only, right?

@MateusStano
Copy link
Member Author

Also I did not care so much about the new jupyter notebook, I think it is there as a fast testing option only, right?

Yes, its just for testing right now

I'll go ahead and merge this then

@MateusStano MateusStano merged commit 2e3b895 into enh/sensors-impl May 21, 2024
10 checks passed
@MateusStano MateusStano deleted the enh/sensors branch May 21, 2024 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Controllers Controlling rocket flight methods Enhancement New feature or request, including adjustments in current codes Flight Flight Class related features
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

Sensors
4 participants