Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
80 changes: 80 additions & 0 deletions docs/user/motors/genericmotor.rst
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,83 @@ note that the user can still provide the parameters manually if needed.
The ``load_from_eng_file`` method is a very useful tool for simulating motors \
when the user does not have all the information required to build a ``SolidMotor`` yet.

The ``load_from_thrustcurve_api`` method
---------------------------------------
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
---------------------------------------
----------------------------------------


The ``GenericMotor`` class provides a convenience loader that downloads a temporary
`.eng` file from the ThrustCurve.org public API and builds a ``GenericMotor``
instance from it. This is useful when you know a motor designation (for example
``"M1670"``) but do not want to manually download and
save the `.eng` file.

.. note::

This method performs network requests to the ThrustCurve API. Use it only
when you have network access. For automated testing or reproducible runs,
prefer using local `.eng` files.
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

Missing blank line before the "Signature" subsection heading. According to reStructuredText conventions, there should be a blank line between the end of a note/admonition block and the next heading.

Suggested change
prefer using local `.eng` files.
prefer using local `.eng` files.

Copilot uses AI. Check for mistakes.
Signature
Comment on lines +122 to +123
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
prefer using local `.eng` files.
Signature
prefer using local `.eng` files.
Signature

----------

``GenericMotor.load_from_thrustcurve_api(name: str, **kwargs) -> GenericMotor``

Parameters
----------
name : str
Motor name to search on ThrustCurve (example:
``"M1670"``).Only shorthand names are accepted (e.g. ``"M1670"``, not
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

Missing space after the closing parenthesis and before "Only".

Suggested change
``"M1670"``).Only shorthand names are accepted (e.g. ``"M1670"``, not
``"M1670"``). Only shorthand names are accepted (e.g. ``"M1670"``, not

Copilot uses AI. Check for mistakes.
``"Cesaroni M1670"``).
when multiple matches occur the first result returned by the API is used.
Comment on lines +131 to +134
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The documentation states "Only shorthand names are accepted (e.g. \"M1670\", not \"Cesaroni M1670\")", but the code docstring at line 1929-1931 says "Both manufacturer-prefixed and shorthand names are commonly used". These statements are contradictory. Based on the code implementation using the "commonName" parameter, both formats should work, so this documentation is inaccurate.

Suggested change
Motor name to search on ThrustCurve (example:
``"M1670"``).Only shorthand names are accepted (e.g. ``"M1670"``, not
``"Cesaroni M1670"``).
when multiple matches occur the first result returned by the API is used.
Motor name to search on ThrustCurve (examples:
``"M1670"`` or ``"Cesaroni M1670"``). Both shorthand and manufacturer-prefixed
names are accepted. When multiple matches occur, the first result returned by
the API is used.

Copilot uses AI. Check for mistakes.
Comment on lines +132 to +134
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The first letter of "when" should be capitalized as "When" since it starts a new sentence. Also missing space after the closing parenthesis.

Suggested change
``"M1670"``).Only shorthand names are accepted (e.g. ``"M1670"``, not
``"Cesaroni M1670"``).
when multiple matches occur the first result returned by the API is used.
``"M1670"``). Only shorthand names are accepted (e.g. ``"M1670"``, not
``"Cesaroni M1670"``).
When multiple matches occur the first result returned by the API is used.

Copilot uses AI. Check for mistakes.
**kwargs :
Same optional arguments accepted by the :class:`GenericMotor` constructor
(e.g. ``dry_mass``, ``nozzle_radius``, ``interpolation_method``). Any
parameters provided here override values parsed from the downloaded file.

Returns
----------
GenericMotor
A new ``GenericMotor`` instance created from the .eng data downloaded from
ThrustCurve.

Raises
----------
ValueError
If the API search returns no motor, or if the download endpoint returns no
.eng file or empty/invalid data.
requests.exceptions.RequestException
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The requests.exceptions.RequestException line is incomplete - it's missing a description of when this exception is raised. It should include text similar to what's in the code docstring: "If a network or HTTP error occurs during the API call."

Suggested change
requests.exceptions.RequestException
requests.exceptions.RequestException
If a network or HTTP error occurs during the API call to ThrustCurve.

Copilot uses AI. Check for mistakes.

Behavior notes
---------------
- The method first performs a search on ThrustCurve using the provided name.
If no results are returned a :class:`ValueError` is raised.
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The phrase "If no results are returned a :class:ValueError is raised" is missing a comma after "returned". It should read: "If no results are returned, a :class:ValueError is raised."

Suggested change
If no results are returned a :class:`ValueError` is raised.
If no results are returned, a :class:`ValueError` is raised.

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

true

- If a motor is found the method requests the .eng file in RASP format, decodes
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The phrase "If a motor is found the method requests" is missing a comma after "found". It should read: "If a motor is found, the method requests..."

Suggested change
- If a motor is found the method requests the .eng file in RASP format, decodes
- If a motor is found, the method requests the .eng file in RASP format, decodes

Copilot uses AI. Check for mistakes.
it and temporarily writes it to disk; a ``GenericMotor`` is then constructed
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The phrase "it and temporarily writes it to disk; a GenericMotor is then constructed" uses a semicolon incorrectly. Consider using a period or comma instead: "it and temporarily writes it to disk. A GenericMotor is then constructed..." or "it and temporarily writes it to disk, and a GenericMotor is then constructed..."

Suggested change
it and temporarily writes it to disk; a ``GenericMotor`` is then constructed
it and temporarily writes it to disk. A ``GenericMotor`` is then constructed

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

true

using the existing .eng file loader. The temporary file is removed even if an
error occurs.
- The function emits a non-fatal informational warning when a motor is found
(``warnings.warn(...)``). This follows the repository convention for
non-critical messages; callers can filter or suppress warnings as needed.

Example
---------------

.. jupyter-execute::

from rocketpy.motors import GenericMotor

# Build a motor by name (requires network access)
motor = GenericMotor.load_from_thrustcurve_api("M1670")

# Use the motor as usual
motor.info()

Testing advice
---------------
- ``pytest``'s ``caplog`` or ``capfd`` to assert on log/warning output.
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The "Testing advice" section is incomplete. It starts with "- pytest's caplog or capfd to assert on log/warning output." which is a sentence fragment. This should be a complete sentence, such as: "- Use pytest's caplog or capfd to assert on log/warning output."

Suggested change
- ``pytest``'s ``caplog`` or ``capfd`` to assert on log/warning output.
- Use ``pytest``'s ``caplog`` or ``capfd`` to assert on log/warning output.

Copilot uses AI. Check for mistakes.

Security & reliability
----------------
- The method makes outgoing HTTP requests and decodes base64-encoded content;
validate inputs in upstream code if you accept motor names from untrusted
sources.
- Network failures, API rate limits, or changes to the ThrustCurve API may
break loading; consider caching downloaded `.eng` files for production use.
120 changes: 119 additions & 1 deletion rocketpy/motors/motor.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import base64
import re
import tempfile
import warnings
import xml.etree.ElementTree as ET
from abc import ABC, abstractmethod
from functools import cached_property
from os import path
from os import path, remove

import numpy as np
import requests

from ..mathutils.function import Function, funcify_method
from ..plots.motor_plots import _MotorPlots
Expand Down Expand Up @@ -1914,6 +1917,121 @@ def load_from_rse_file(
coordinate_system_orientation=coordinate_system_orientation,
)

@staticmethod
def call_thrustcurve_api(name: str):
Comment on lines +1920 to +1921
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The call_thrustcurve_api method is marked as @staticmethod but it's only called from load_from_thrustcurve_api at line 2013 using GenericMotor.call_thrustcurve_api(name). This method could be marked as a private method (e.g., _call_thrustcurve_api) since it's an internal implementation detail not intended for public use. The docstring doesn't indicate it's part of the public API, and the method name doesn't follow common naming patterns for public utility methods.

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

I agree

Copy link
Collaborator

Choose a reason for hiding this comment

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

I also agree, I would not leave this method as a public member of the class. A simple _ should fix it.

"""
Download a .eng file from the ThrustCurve API
based on the given motor name.

Parameters
----------
name : str
The motor name according to the API (e.g., "Cesaroni_M1670" or "M1670").
Both manufacturer-prefixed and shorthand names are commonly used; if multiple
motors match the search, the first result is used.

Returns
-------
data_base64 : String
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The return type should be str (lowercase) instead of String (capitalized). Python's built-in string type is documented as str in type hints and docstrings.

Suggested change
data_base64 : String
data_base64 : str

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

true

The .eng file of the motor in base64

Raises
------
ValueError
If no motor is found or if the downloaded .eng data is missing.
requests.exceptions.RequestException
If a network or HTTP error occurs during the API call.
"""
base_url = "https://www.thrustcurve.org/api/v1"

# Step 1. Search motor
response = requests.get(f"{base_url}/search.json", params={"commonName": name})
response.raise_for_status()
data = response.json()

if not data.get("results"):
raise ValueError(
f"No motor found for name '{name}'. "
"Please verify the motor name format (e.g., 'Cesaroni_M1670' or 'M1670') and try again."
)

motor_info = data["results"][0]
motor_id = motor_info.get("motorId")
designation = motor_info.get("designation", "").replace("/", "-")
manufacturer = motor_info.get("manufacturer", "")
warnings.warn(f"Motor found: {designation} ({manufacturer})", UserWarning)

# Step 2. Download the .eng file
dl_response = requests.get(
f"{base_url}/download.json",
params={"motorIds": motor_id, "format": "RASP", "data": "file"},
)
dl_response.raise_for_status()
dl_data = dl_response.json()

if not dl_data.get("results"):
raise ValueError(
f"No .eng file found for motor '{name}' in the ThrustCurve API."
)

data_base64 = dl_data["results"][0].get("data")
if not data_base64:
raise ValueError(
f"Downloaded .eng data for motor '{name}' is empty or invalid."
)
return data_base64

@staticmethod
def load_from_thrustcurve_api(name: str, **kwargs):
"""
Creates a Motor instance by downloading a .eng file from the ThrustCurve API
based on the given motor name.

Parameters
----------
name : str
The motor name according to the API (e.g., "Cesaroni_M1670" or "M1670").
Both manufacturer-prefixed and shorthand names are commonly used; if multiple
motors match the search, the first result is used.
**kwargs :
Additional arguments passed to the Motor constructor or loader, such as
dry_mass, nozzle_radius, etc.

Returns
-------
instance : GenericMotor
A new GenericMotor instance initialized using the downloaded .eng file.

Raises
------
ValueError
If no motor is found or if the downloaded .eng data is missing.
requests.exceptions.RequestException
If a network or HTTP error occurs during the API call.
"""

data_base64 = GenericMotor.call_thrustcurve_api(name)
data_bytes = base64.b64decode(data_base64)

# Step 3. Create the motor from the .eng file
tmp_path = None
try:
# create a temporary file that persists until we explicitly remove it
with tempfile.NamedTemporaryFile(suffix=".eng", delete=False) as tmp_file:
tmp_file.write(data_bytes)
tmp_file.flush()
tmp_path = tmp_file.name

return GenericMotor.load_from_eng_file(tmp_path, **kwargs)
finally:
# Ensuring the temporary file is removed
if tmp_path and path.exists(tmp_path):
try:
remove(tmp_path)
except OSError:
# If cleanup fails, don't raise: we don't want to mask prior exceptions.
pass

def all_info(self):
"""Prints out all data and graphs available about the Motor."""
# Print motor details
Expand Down
114 changes: 114 additions & 0 deletions tests/unit/motors/test_genericmotor.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import base64

import numpy as np
import pytest
import requests
import scipy.integrate

from rocketpy import Function, Motor
Expand Down Expand Up @@ -211,3 +214,114 @@ def test_load_from_rse_file(generic_motor):
assert thrust_curve[0][1] == 0.0 # First thrust point
assert thrust_curve[-1][0] == 2.2 # Last point of time
assert thrust_curve[-1][1] == 0.0 # Last thrust point


class MockResponse:
"""Mocked response for requests."""

def __init__(self, json_data):
self._json_data = json_data

def json(self):
return self._json_data

def raise_for_status(self):
return None


def _mock_get(search_results=None, download_results=None):
"""Return a mock_get function with predefined search/download results."""

def _get(url, **_kwargs):
if "search.json" in url:
return MockResponse(search_results or {"results": []})
if "download.json" in url:
return MockResponse(download_results or {"results": []})
raise RuntimeError(f"Unexpected URL: {url}")

return _get


def assert_motor_specs(motor):
burn_time = (0, 3.9)
dry_mass = 2.130
propellant_initial_mass = 3.101
chamber_radius = 75 / 1000
chamber_height = 757 / 1000
nozzle_radius = chamber_radius * 0.85
average_thrust = 1545.218
total_impulse = 6026.350
max_thrust = 2200.0
exhaust_velocity = 1943.357

assert motor.burn_time == burn_time
assert motor.dry_mass == dry_mass
assert motor.propellant_initial_mass == propellant_initial_mass
assert motor.chamber_radius == chamber_radius
assert motor.chamber_height == chamber_height
assert motor.chamber_position == 0
assert motor.average_thrust == pytest.approx(average_thrust)
assert motor.total_impulse == pytest.approx(total_impulse)
assert motor.exhaust_velocity.average(*burn_time) == pytest.approx(exhaust_velocity)
assert motor.max_thrust == pytest.approx(max_thrust)
assert motor.nozzle_radius == pytest.approx(nozzle_radius)


Comment on lines +245 to +269
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The assert_motor_specs function duplicates the same test values found in the test_load_from_eng_file function (lines 148-159). This creates maintenance burden - if these expected values need to be updated, they must be changed in two places. Consider extracting these as module-level constants or creating a fixture that can be reused by both functions.

Suggested change
def assert_motor_specs(motor):
burn_time = (0, 3.9)
dry_mass = 2.130
propellant_initial_mass = 3.101
chamber_radius = 75 / 1000
chamber_height = 757 / 1000
nozzle_radius = chamber_radius * 0.85
average_thrust = 1545.218
total_impulse = 6026.350
max_thrust = 2200.0
exhaust_velocity = 1943.357
assert motor.burn_time == burn_time
assert motor.dry_mass == dry_mass
assert motor.propellant_initial_mass == propellant_initial_mass
assert motor.chamber_radius == chamber_radius
assert motor.chamber_height == chamber_height
assert motor.chamber_position == 0
assert motor.average_thrust == pytest.approx(average_thrust)
assert motor.total_impulse == pytest.approx(total_impulse)
assert motor.exhaust_velocity.average(*burn_time) == pytest.approx(exhaust_velocity)
assert motor.max_thrust == pytest.approx(max_thrust)
assert motor.nozzle_radius == pytest.approx(nozzle_radius)
# Module-level constant for expected motor specs
EXPECTED_MOTOR_SPECS = {
"burn_time": (0, 3.9),
"dry_mass": 2.130,
"propellant_initial_mass": 3.101,
"chamber_radius": 75 / 1000,
"chamber_height": 757 / 1000,
"nozzle_radius": (75 / 1000) * 0.85,
"average_thrust": 1545.218,
"total_impulse": 6026.350,
"max_thrust": 2200.0,
"exhaust_velocity": 1943.357,
"chamber_position": 0,
}
def assert_motor_specs(motor):
specs = EXPECTED_MOTOR_SPECS
assert motor.burn_time == specs["burn_time"]
assert motor.dry_mass == specs["dry_mass"]
assert motor.propellant_initial_mass == specs["propellant_initial_mass"]
assert motor.chamber_radius == specs["chamber_radius"]
assert motor.chamber_height == specs["chamber_height"]
assert motor.chamber_position == specs["chamber_position"]
assert motor.average_thrust == pytest.approx(specs["average_thrust"])
assert motor.total_impulse == pytest.approx(specs["total_impulse"])
assert motor.exhaust_velocity.average(*specs["burn_time"]) == pytest.approx(specs["exhaust_velocity"])
assert motor.max_thrust == pytest.approx(specs["max_thrust"])
assert motor.nozzle_radius == pytest.approx(specs["nozzle_radius"])

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

true

def test_load_from_thrustcurve_api(monkeypatch, generic_motor):
"""Tests GenericMotor.load_from_thrustcurve_api with mocked API."""

eng_path = "data/motors/cesaroni/Cesaroni_M1670.eng"
with open(eng_path, "rb") as f:
encoded = base64.b64encode(f.read()).decode("utf-8")

search_json = {
"results": [
{
"motorId": "12345",
"designation": "Cesaroni_M1670",
"manufacturer": "Cesaroni",
}
]
}
download_json = {"results": [{"data": encoded}]}
monkeypatch.setattr(requests, "get", _mock_get(search_json, download_json))
monkeypatch.setattr(requests.Session, "get", _mock_get(search_json, download_json))

motor = type(generic_motor).load_from_thrustcurve_api("M1670")

assert_motor_specs(motor)

_, _, points = Motor.import_eng(eng_path)
assert motor.thrust.y_array == pytest.approx(
Function(points, "Time (s)", "Thrust (N)", "linear", "zero").y_array
)

error_cases = [
("No motor found", {"results": []}, None),
(
"No .eng file found",
{
"results": [
{"motorId": "123", "designation": "Fake", "manufacturer": "Test"}
]
},
{"results": []},
),
(
"Downloaded .eng data",
{
"results": [
{"motorId": "123", "designation": "Fake", "manufacturer": "Test"}
]
},
{"results": [{"data": ""}]},
),
]

for msg, search_res, download_res in error_cases:
monkeypatch.setattr(requests, "get", _mock_get(search_res, download_res))
monkeypatch.setattr(
requests.Session, "get", _mock_get(search_res, download_res)
)
with pytest.raises(ValueError, match=msg):
type(generic_motor).load_from_thrustcurve_api("FakeMotor")
Loading