-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add test and Docs for simple motor #282
base: master
Are you sure you want to change the base?
Conversation
Joao, do you want me to review it? |
for sure ! |
I've already noticed some imports that are irrelevant for the code, that i will fix it in the next commit, but besides that would be glad to know if there's anything to improve. |
Will do! |
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.
Just small changes to do. PR is very good. When you send the docs, please remove the WIP tag, link the corresponding issue, and request the reviews.
def test_get_thrust(motor, plane): | ||
|
||
motor.set_parent(plane) | ||
x,y = motor.get_thrust() | ||
|
||
check_thrust_center = Vector2(0.3, 0) | ||
check_thrust = Vector2(82.26940810871174, 0) | ||
|
||
npt.assert_array_almost_equal(x, check_thrust) | ||
npt.assert_array_almost_equal(y, check_thrust_center) |
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.
Another nice assert would be to incline the plane and assert that the thrust also inclines by the same value (that is, that the implementation assures that the thrust is aligned with the component axis).
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.
Sorry, but i'm not quite sure how to implement this.
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.
Adding this after your previous code:
plane.angle = math.radians(15)
x,y = motor.get_thrust()
check_thrust_center = Vector2(0.3, 0)
check_thrust = Vector2(82.26940810871174, 0)
npt.assert_array_almost_equal(x, check_thrust)
npt.assert_array_almost_equal(y, check_thrust_center)
Basically, you are checking if the thrust remains the same if the plane is inclined (15 degrees) or not (0 degrees). It should be, since the get_thrust() method returns the thrust in the motor frame of reference.
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.
Perfect. Will upload later a commit with this.
@@ -0,0 +1,57 @@ | |||
# Simple Motor | |||
|
|||
Simple Motor class is based on the class called "attatched component", and written over the Motor class. For the fact that it is an attatched component, it is needed to set a parent for it, and just like on attached component, all information from the parent will be taken in count on the children methods. |
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 the Motor class, which inherits from the AttachedComponent class, ...
|
||
It's a property that returns a Vector2 with the values of the points where the thrust is applied. | ||
|
||
As it is a Simple Motor, by default the force is applied on x in the distance from the origin to the propeller and in y, in 0 as it's seen in the example below: |
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.
As it is a simplified model of a motor, by default the force is applied on the motor axis (y = 0), on a given distance (distance_origin_to_propeller) over the horizontal X-axis, as it's seen in the example below:
* linear_coefficient | ||
* distance_origin_to_propeller | ||
|
||
## *Thrust Center* _method_: |
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.
better to use the same name as the method (thrust_center).
print(motor.thrust_center) | ||
>>> Vector2(0.3, 0) | ||
``` | ||
##*Get Thrust* method: |
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.
get_thrust
``` | ||
##*Get Thrust* method: | ||
|
||
This methods is responsible for returning the magnitude value of the given thrust. To make the calculation, the method calls the function get_axial_thrust_from_linear_model, that takes into consideration the motor inputs, the environment variables, and the given velocity of the aircraft rotated to the axial velocity, considering the angle of attack. |
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.
It returns the magnitude of the thrust and the point where it is applied.
...and the given velocity of the aircraft rotated to the axial velocity, considering the angle of attack.
can be better described this way:
...and the component of the aircraft velocity aligned with the motor axis.
|
||
This methods is responsible for returning the magnitude value of the given thrust. To make the calculation, the method calls the function get_axial_thrust_from_linear_model, that takes into consideration the motor inputs, the environment variables, and the given velocity of the aircraft rotated to the axial velocity, considering the angle of attack. | ||
|
||
The method returns to vectors, one with the thrust magnitude and the other one with the thrust_center, that was calculated on the method before. |
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.
This can be merged with the first phrase.
def test_thrust_center(motor, plane): | ||
|
||
motor.set_parent(plane) | ||
|
||
check_thrust_center = Vector2(0.3, 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.
Excess of blank lines
assert(motor.thrust_center == check_thrust_center) | ||
|
||
def test_get_thrust(motor, plane): | ||
|
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.
You can remove this blank line
x,y = motor.get_thrust() | ||
|
||
check_thrust_center = Vector2(0.3, 0) | ||
check_thrust = Vector2(82.269378, 0) | ||
|
||
npt.assert_array_almost_equal(x, check_thrust) | ||
npt.assert_array_almost_equal(y, check_thrust_center) |
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.
x and y are bad names, not recognizable. You could use thrust_center and thrust, as you use right after.
Fixes #271