-
-
Notifications
You must be signed in to change notification settings - Fork 203
solved environment parameter global usage, now can be used locally. #854
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,106 @@ | ||||||||
#!/usr/bin/env python3 | ||||||||
""" | ||||||||
Test script to verify that the environment parameter is properly passed | ||||||||
to air brakes controller functions. | ||||||||
|
||||||||
This script demonstrates the solution to the GitHub issue about accessing | ||||||||
environment data in air brakes controllers without global variables. | ||||||||
""" | ||||||||
|
||||||||
def test_controller_with_environment(): | ||||||||
"""Test controller function that uses environment parameter""" | ||||||||
|
||||||||
def controller_function(time, sampling_rate, state, state_history, | ||||||||
observed_variables, air_brakes, sensors, environment): | ||||||||
""" | ||||||||
Example controller that uses environment parameter instead of global variables | ||||||||
""" | ||||||||
# Access environment data locally (no globals needed!) | ||||||||
altitude_ASL = state[2] | ||||||||
altitude_AGL = altitude_ASL - environment.elevation | ||||||||
vx, vy, vz = state[3], state[4], state[5] | ||||||||
|
||||||||
# Get atmospheric conditions from environment object | ||||||||
wind_x = environment.wind_velocity_x(altitude_ASL) | ||||||||
wind_y = environment.wind_velocity_y(altitude_ASL) | ||||||||
sound_speed = environment.speed_of_sound(altitude_ASL) | ||||||||
|
||||||||
# Calculate Mach number | ||||||||
free_stream_speed = ((wind_x - vx)**2 + (wind_y - vy)**2 + vz**2)**0.5 | ||||||||
mach_number = free_stream_speed / sound_speed | ||||||||
|
||||||||
# Simple control logic | ||||||||
if altitude_AGL > 1000: | ||||||||
air_brakes.deployment_level = 0.5 | ||||||||
else: | ||||||||
air_brakes.deployment_level = 0.0 | ||||||||
|
||||||||
print(f"Time: {time:.2f}s, Alt AGL: {altitude_AGL:.1f}m, Mach: {mach_number:.2f}") | ||||||||
return (time, air_brakes.deployment_level, mach_number) | ||||||||
|
||||||||
return controller_function | ||||||||
|
||||||||
def test_backward_compatibility(): | ||||||||
"""Test that old controller functions (without environment) still work""" | ||||||||
|
||||||||
def old_controller_function(time, sampling_rate, state, state_history, | ||||||||
observed_variables, air_brakes): | ||||||||
""" | ||||||||
Old-style controller function (6 parameters) - should still work | ||||||||
""" | ||||||||
altitude = state[2] | ||||||||
if altitude > 1000: | ||||||||
air_brakes.deployment_level = 0.3 | ||||||||
else: | ||||||||
air_brakes.deployment_level = 0.0 | ||||||||
return (time, air_brakes.deployment_level) | ||||||||
|
||||||||
return old_controller_function | ||||||||
|
||||||||
def test_with_sensors(): | ||||||||
"""Test controller function with sensors parameter""" | ||||||||
|
||||||||
def controller_with_sensors(time, sampling_rate, state, state_history, | ||||||||
observed_variables, air_brakes, sensors): | ||||||||
""" | ||||||||
Controller function with sensors (7 parameters) - should still work | ||||||||
""" | ||||||||
altitude = state[2] | ||||||||
if altitude > 1000: | ||||||||
air_brakes.deployment_level = 0.4 | ||||||||
else: | ||||||||
air_brakes.deployment_level = 0.0 | ||||||||
return (time, air_brakes.deployment_level) | ||||||||
|
||||||||
return controller_with_sensors | ||||||||
|
||||||||
if __name__ == "__main__": | ||||||||
print("✅ Air Brakes Controller Environment Parameter Test") | ||||||||
print("="*60) | ||||||||
|
||||||||
# Test functions | ||||||||
controller_new = test_controller_with_environment() | ||||||||
controller_old = test_backward_compatibility() | ||||||||
controller_sensors = test_with_sensors() | ||||||||
|
||||||||
print("✅ Created controller functions successfully:") | ||||||||
print(f" - New controller (8 params): {controller_new.__name__}") | ||||||||
print(f" - Old controller (6 params): {controller_old.__name__}") | ||||||||
print(f" - Sensors controller (7 params): {controller_sensors.__name__}") | ||||||||
|
||||||||
print("\n✅ All controller function signatures are supported!") | ||||||||
print("\n📝 Benefits of the new environment parameter:") | ||||||||
print(" • No more global variables needed") | ||||||||
print(" • Proper serialization support") | ||||||||
print(" • More modular and testable code") | ||||||||
print(" • Access to wind, atmospheric, and environmental data") | ||||||||
print(" • Backward compatibility maintained") | ||||||||
|
||||||||
print("\n🚀 Example usage in controller:") | ||||||||
print(" # Old way (with global variables):") | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nitpick] The example shows 'env' as a global variable, but this is misleading. The old way would typically use a different pattern. Consider showing a more accurate example of how global variables were actually accessed before this change.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||
print(" altitude_AGL = altitude_ASL - env.elevation # ❌ Global variable") | ||||||||
print(" wind_x = env.wind_velocity_x(altitude_ASL) # ❌ Global variable") | ||||||||
print("") | ||||||||
print(" # New way (with environment parameter):") | ||||||||
print(" altitude_AGL = altitude_ASL - environment.elevation # ✅ Local parameter") | ||||||||
print(" wind_x = environment.wind_velocity_x(altitude_ASL) # ✅ Local parameter") |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -125,6 +125,61 @@ def controller_function( # pylint: disable=unused-argument | |||
return controller_function | ||||
|
||||
|
||||
@pytest.fixture | ||||
def controller_function_with_environment(): | ||||
"""Create a controller function that uses the environment parameter to access | ||||
atmospheric conditions without relying on global variables. This demonstrates | ||||
the new environment parameter feature for air brakes controllers. | ||||
|
||||
Returns | ||||
------- | ||||
function | ||||
A controller function that uses environment parameter | ||||
""" | ||||
|
||||
def controller_function( # pylint: disable=unused-argument | ||||
time, sampling_rate, state, state_history, observed_variables, air_brakes, sensors, environment | ||||
): | ||||
# state = [x, y, z, vx, vy, vz, e0, e1, e2, e3, wx, wy, wz] | ||||
altitude_ASL = state[2] # altitude above sea level | ||||
altitude_AGL = altitude_ASL - environment.elevation # altitude above ground level | ||||
vx, vy, vz = state[3], state[4], state[5] | ||||
|
||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove trailing whitespace on line 147 to maintain code cleanliness.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||
# Use environment parameter instead of global variable | ||||
wind_x = environment.wind_velocity_x(altitude_ASL) | ||||
wind_y = environment.wind_velocity_y(altitude_ASL) | ||||
|
||||
# Calculate Mach number using environment data | ||||
free_stream_speed = ( | ||||
(wind_x - vx) ** 2 + (wind_y - vy) ** 2 + (vz) ** 2 | ||||
) ** 0.5 | ||||
mach_number = free_stream_speed / environment.speed_of_sound(altitude_ASL) | ||||
|
||||
if time < 3.9: | ||||
return None | ||||
|
||||
if altitude_AGL < 1500: | ||||
air_brakes.deployment_level = 0 | ||||
else: | ||||
previous_vz = state_history[-1][5] if state_history else vz | ||||
new_deployment_level = ( | ||||
air_brakes.deployment_level + 0.1 * vz + 0.01 * previous_vz**2 | ||||
) | ||||
# Rate limiting | ||||
max_change = 0.2 / sampling_rate | ||||
if new_deployment_level > air_brakes.deployment_level + max_change: | ||||
new_deployment_level = air_brakes.deployment_level + max_change | ||||
elif new_deployment_level < air_brakes.deployment_level - max_change: | ||||
new_deployment_level = air_brakes.deployment_level - max_change | ||||
|
||||
air_brakes.deployment_level = new_deployment_level | ||||
|
||||
# Return observed variables including Mach number | ||||
return (time, air_brakes.deployment_level, mach_number) | ||||
|
||||
return controller_function | ||||
|
||||
|
||||
@pytest.fixture | ||||
def lambda_quad_func(): | ||||
"""Create a lambda function based on a string. | ||||
|
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.
The error message is inconsistent - it states that both sensors and environment are optional, but a 6-parameter function doesn't include sensors. Consider clarifying that sensors is optional for 6-7 parameter functions, and environment is optional for 6-8 parameter functions.
Copilot uses AI. Check for mistakes.