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.

Sign up for GitHub

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

Implements rocket route tests #40

Merged
merged 5 commits into from
Nov 23, 2024
Merged

Implements rocket route tests #40

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c0b4aba
updates pylint ignore rules
GabrielBarberini Nov 16, 2024
fdb0f7c
adds rocket route tests
GabrielBarberini Nov 16, 2024
4065960
Update pylint.yml
GabrielBarberini Nov 16, 2024
0b79762
updates mermaid flowchart
GabrielBarberini Nov 16, 2024
dcb54b8
addresses PR review
GabrielBarberini Nov 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ flake8:

pylint:
pylint ./lib || true
pylint --disable=E0401,W0621 ./tests || true
pylint --disable=E0401,W0621,R0913,R0917 ./tests || true

ruff:
ruff check --fix ./lib || true
Expand Down
2 changes: 1 addition & 1 deletion lib/controllers/flight.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ async def get_rocketpy_flight_binary(

@classmethod
async def update_flight_by_id(
cls, flight: Flight, flight_id: str
cls, flight_id: str, flight: Flight
) -> Union[FlightUpdated, HTTPException]:
"""
Update a models.Flight in the database.
Expand Down
2 changes: 1 addition & 1 deletion lib/controllers/rocket.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ async def get_rocketpy_rocket_binary(

@classmethod
async def update_rocket_by_id(
cls, rocket: Rocket, rocket_id: str
cls, rocket_id: str, rocket: Rocket
) -> Union[RocketUpdated, HTTPException]:
"""
Update a models.Rocket in the database.
Expand Down
11 changes: 10 additions & 1 deletion lib/models/aerosurfaces.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
from enum import Enum
from typing import Optional, Tuple, List
from typing import Optional, Tuple, List, Union
from pydantic import BaseModel


class Parachute(BaseModel):
name: str
cd_s: float
sampling_rate: int
Copy link
Member

Choose a reason for hiding this comment

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

almost sure the sampling_rate can be a float value

lag: float
trigger: Union[str, float]
noise: Tuple[float, float, float]
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add field validations and improve documentation

The Parachute class needs additional validations and documentation:

  1. Field validations:

    • cd_s should be positive (drag coefficient × surface area)
    • sampling_rate should be positive
    • lag should be non-negative

  2. Documentation needs:

    • Class docstring explaining purpose and usage
    • Document valid values for trigger field
    • Document what each float in noise tuple represents

Here's a suggested implementation:

 class Parachute(BaseModel):
+    """Represents a parachute aerosurface with deployment characteristics.
+
+    Attributes:
+        name: Identifier for the parachute
+        cd_s: Product of drag coefficient and surface area (m²)
+        sampling_rate: Data sampling rate in Hz
+        lag: Deployment delay in seconds
+        trigger: Deployment condition (altitude in meters or event name)
+        noise: Simulation noise parameters (mean, std, time_correlation)
+    """
     name: str
-    cd_s: float
-    sampling_rate: int
-    lag: float
+    cd_s: float = Field(..., gt=0, description="Must be positive")
+    sampling_rate: int = Field(..., gt=0, description="Must be positive")
+    lag: float = Field(..., ge=0, description="Must be non-negative")
     trigger: Union[str, float]
     noise: Tuple[float, float, float]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
class Parachute(BaseModel):
name: str
cd_s: float
sampling_rate: int
lag: float
trigger: Union[str, float]
noise: Tuple[float, float, float]
class Parachute(BaseModel):
"""Represents a parachute aerosurface with deployment characteristics.
Attributes:
name: Identifier for the parachute
cd_s: Product of drag coefficient and surface area (m²)
sampling_rate: Data sampling rate in Hz
lag: Deployment delay in seconds
trigger: Deployment condition (altitude in meters or event name)
noise: Simulation noise parameters (mean, std, time_correlation)
"""
name: str
cd_s: float = Field(..., gt=0, description="Must be positive")
sampling_rate: int = Field(..., gt=0, description="Must be positive")
lag: float = Field(..., ge=0, description="Must be non-negative")
trigger: Union[str, float]
noise: Tuple[float, float, float]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Gui-FernandesBR do you agree with that ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Member

Choose a reason for hiding this comment

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

@GabrielBarberini yes the idea is great, just use ge instead of gt for cd_s. The cd_s can also be zero.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

patched here dcb54b8



class RailButtons(BaseModel):
name: str = "RailButtons"
upper_button_position: float
Expand Down
10 changes: 1 addition & 9 deletions lib/models/rocket.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
NoseCone,
Tail,
RailButtons,
Parachute,
)


Expand All @@ -15,15 +16,6 @@ class CoordinateSystemOrientation(str, Enum):
NOSE_TO_TAIL: str = "NOSE_TO_TAIL"


class Parachute(BaseModel):
name: str
cd_s: float
sampling_rate: int
lag: float
trigger: Union[str, float]
noise: Tuple[float, float, float]


class Rocket(BaseModel):
# Required parameters
motor: Motor
Expand Down
4 changes: 2 additions & 2 deletions lib/routes/flight.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ async def read_flight(flight_id: str) -> FlightView:


@router.get(
"/rocketpy/{flight_id}",
"/{flight_id}/rocketpy",
responses={
203: {
"description": "Binary file download",
Expand Down Expand Up @@ -146,7 +146,7 @@ async def update_flight(
"""
with tracer.start_as_current_span("update_flight"):
flight.rocket.motor.set_motor_kind(motor_kind)
return await FlightController.update_flight_by_id(flight, flight_id)
return await FlightController.update_flight_by_id(flight_id, flight)


@router.get("/{flight_id}/summary")
Expand Down
4 changes: 2 additions & 2 deletions lib/routes/rocket.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,11 @@ async def update_rocket(
"""
with tracer.start_as_current_span("update_rocket"):
rocket.motor.set_motor_kind(motor_kind)
return await RocketController.update_rocket_by_id(rocket, rocket_id)
return await RocketController.update_rocket_by_id(rocket_id, rocket)


@router.get(
"/rocketpy/{rocket_id}",
"/{rocket_id}/rocketpy",
GabrielBarberini marked this conversation as resolved.
Show resolved Hide resolved
responses={
203: {
"description": "Binary file download",
Expand Down
Loading
Loading