Conversation
There was a problem hiding this comment.
Pull request overview
Refactors Avionics identifiers to align with the newly documented naming guidelines in AGENTS.md, updating constants, methods, and variables across state-estimation, data-handling, UART CLI, and associated unit tests.
Changes:
- Renamed state-estimation APIs/fields (e.g.,
ApogeePredictor::*Updatemethods,VerticalVelocityEstimatorstate fields,LaunchDetectorinternals). - Renamed data-handling constants and helpers (telemetry packet sizing/serialization helpers; DataSaver SPI/SD constants).
- Updated tests and
AGENTS.mdto match the new naming/units conventions.
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_vertical_velocity_estimator/test_vve_csv.h | Updates VVE test to use renamed time conversion constant. |
| test/test_vertical_velocity_estimator/main.cpp | Updates/clarifies a test comment after naming refactor. |
| test/test_telemetry_data_transmission/test_telemetry_data_transmission.cpp | Updates telemetry header-size constant name in comments. |
| test/test_datasaver_spi/test_DataSaverSPI.cpp | Updates DataSaverSPI constant names used by tests. |
| test/test_circular_array/test_circular_array.h | Updates CircularArray capacity constant name used by tests. |
| test/test_burnout_state_machine/test_burnout_state_machine_csv.cpp | Updates ApogeePredictor method calls to new names. |
| test/test_apogee_predictor_sim/main.cpp | Updates ApogeePredictor method pointer and getter names. |
| src/state_estimation/VerticalVelocityEstimator.cpp | Renames internal state fields and time conversion constant usage. |
| src/state_estimation/LaunchDetector.cpp | Renames window/member variables and related logic references. |
| src/state_estimation/BurnoutStateMachine.cpp | Removes unused globals (cleanup) as part of refactor. |
| src/state_estimation/ApogeePredictor.cpp | Renames constants/methods and some local identifiers. |
| src/state_estimation/ApogeeDetector.cpp | Renames apogee flag variable and references. |
| src/data_handling/Telemetry.cpp | Updates to renamed TelemetryFmt constants and helper function. |
| src/data_handling/DataSaverSPI.cpp | Updates to renamed SPI datasaver constants and buffer sizing. |
| src/data_handling/DataSaverBigSD.cpp | Updates to renamed SD datasaver constants (MiB/bytes). |
| src/UARTCommandHandler.cpp | Renames CLI constants/fields and updates prompt constant usage. |
| include/state_estimation/VerticalVelocityEstimator.h | Renames constants and private state fields. |
| include/state_estimation/LaunchDetector.h | Renames constants, window member, and related accessors. |
| include/state_estimation/ApogeePredictor.h | Renames update methods + drag-coefficient getter. |
| include/state_estimation/ApogeeDetector.h | Renames apogee flag member. |
| include/data_handling/Telemetry.h | Renames packet layout constants and helper functions. |
| include/data_handling/DataSaverSPI.h | Replaces macros with constexpr constants; renames buffer constant. |
| include/data_handling/DataSaverBigSD.h | Renames constants to clarify units; renames buffer-size constant. |
| include/data_handling/CircularArray.h | Renames max-capacity constant. |
| include/UARTCommandHandler.h | Replaces prompt macro with constexpr and renames size constants. |
| AGENTS.md | Adds/records variable naming rules for the repository. |
| .DS_Store | Included in PR metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 45 changed files in this pull request and generated 9 comments.
Comments suppressed due to low confidence (1)
src/state_estimation/ApogeePredictor.cpp:181
- In
analyticUpdate(),gravityis a physical quantity but the name has no unit suffix, which conflicts with the unit-suffix guidance inAGENTS.md(e.g.,velocity_mps). Please rename this to include units (and optionally reuse the shared gravity constant) to make it unambiguous.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 45 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 45 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 45 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 45 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
src/state_estimation/ApogeePredictor.cpp:177
- Local constant
gravityrepresents acceleration due to gravity; per naming conventions, it should include a unit suffix (e.g.,gravity_mps2/kGravity_mps2) and ideally reuse the existingkGravity_mps2constant already defined in this file.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 45 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
Massive refactoring of the variable names in Avionics to be consistent with the naming guidelines in
AGENTS.mdThe new guidelines are below:
Naming Conventions
Use
camelCasefor variables and functions.Use
PascalCasefor class and struct names.Use
kConstantNamefor constants.Use a trailing underscore for private member variables (for example,
privateVar_andprivateTimestamp_ms_).Add unit suffixes with an underscore when a name represents a physical quantity, time value, size, rate, or other semantically meaningful measured quantity (for example,
timestamp_ms,altitude_m,velocity_mps,packetSize_bytes).Prefer exact units like
_bytesand_bitswhen the exact count matters.Use unambiguous binary storage suffixes like
_KiBand_MiBwhen a quantity is naturally expressed that way.Use unambiguous data-rate suffixes like
_kbpsand_Mbps.Avoid ambiguous suffixes such as
_MB,_mB, or_Mbunless the meaning is completely clear in context.Avoid abbreviations unless they are widely understood in the project context or domain (for example,
imu,gps,crc,uart).Prefer descriptive names over shortened ones when there is any chance of confusion.
Motivation
Testing
Check all that apply:
Builds on MARTHA 1.4 after changing the function call for
analyticUpdateChecklist