Added new GNSSSignalType and GNSSSignalsMessage definitions. #384
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| * @return The corresponding string name. | ||
| */ | ||
| P1_CONSTEXPR_FUNC const char* to_string(SatelliteType type) { | ||
| P1_CONSTEXPR_FUNC const char* to_string(GNSSSignalType type) { |
Check warning
Code scanning / CodeQL
Poorly documented large function Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
In general, to fix a “poorly documented large function” warning, you increase meaningful documentation density: add a more detailed function‑level comment describing behavior, invariants, edge cases, and maintenance expectations, and, if useful, a small number of inline comments to highlight non-obvious logic. You avoid redundant comments that simply restate what each line of code already expresses.
For this function, the best fix without altering functionality is:
- Expand the existing Doxygen block above
to_string(GNSSSignalType type)to describe:- That the function maps each
GNSSSignalTypeto a stable, human‑readable identifier. - That it must be kept in sync with the
GNSSSignalTypeenum definition. - What happens for unknown or unsupported values (returns
"INVALID"). - Any usage notes (e.g., suitable for logging, not for user‑facing localization).
- That the function maps each
- Add a brief inline comment near the
switchand/or the default return explaining that"INVALID"is a fallback for values not covered in the switch and that the mapping is intentionally exhaustive.
All changes are confined to src/point_one/fusion_engine/messages/signal_defs.h around lines 1324–1508. No new methods, imports, or definitions are needed; only comments are added.
| @@ -1324,11 +1324,26 @@ | ||
| /** | ||
| * @brief Get a string representation of the @ref GNSSSignalType enum value. | ||
| * | ||
| * This function converts a @ref GNSSSignalType value to a stable, human-readable | ||
| * identifier string intended for logging, debugging, and serialization of | ||
| * signal types. The returned strings are meant to closely match the enum | ||
| * identifiers and are not localized. | ||
| * | ||
| * @note When new values are added to @ref GNSSSignalType, this function must | ||
| * be updated to keep the mapping exhaustive. Callers should not rely on | ||
| * any implicit ordering, only on the string labels documented here. | ||
| * | ||
| * Any value not explicitly handled by the switch statement below will result | ||
| * in the fallback string "INVALID", which indicates an unknown or unsupported | ||
| * signal type. | ||
| * | ||
| * @param type The enum to get the string name for. | ||
| * | ||
| * @return The corresponding string name. | ||
| * @return The corresponding string name, or "INVALID" if @p type is not | ||
| * recognized. | ||
| */ | ||
| P1_CONSTEXPR_FUNC const char* to_string(GNSSSignalType type) { | ||
| // Maintain this switch in sync with the GNSSSignalType enum definition. | ||
| switch (type) { | ||
| case GNSSSignalType::GPS_L1CA: | ||
| return "GPS_L1CA"; | ||
| @@ -1504,6 +1515,7 @@ | ||
| case GNSSSignalType::UNKNOWN: | ||
| return "UNKNOWN"; | ||
| } | ||
| // Fallback for unknown or unsupported GNSSSignalType values. | ||
| return "INVALID"; | ||
| } | ||
|
|
| * | ||
| * @return The corresponding string. | ||
| */ | ||
| P1_CONSTEXPR_FUNC const char* ToPrettyString(GNSSSignalType type) { |
Check warning
Code scanning / CodeQL
Poorly documented large function Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
In general, to fix this type of issue, you increase the amount and quality of comments in the large function so that its intent, structure, and any non-obvious design choices are clear. The goal is not to comment every line, but to provide enough documentation that a maintainer can quickly understand how the function is organized and how to safely add or modify cases.
For ToPrettyString(GNSSSignalType type) in src/point_one/fusion_engine/messages/signal_defs.h, the best fix without changing functionality is:
- Keep the existing brief Doxygen comment as-is.
- Add a short, high-level comment at the top of the function body explaining that it maps each
GNSSSignalTypeto a concise, human-friendly label suitable for logs/UI, and that the mapping must stay in sync with the enum definition. - Within the
switch, add block comments to delineate logical groups of signals (e.g., GPS, GLONASS, Galileo, BeiDou, SBAS, etc.), assuming the cases are already grouped that way. This improves readability and makes it clearer where to add new cases for a given constellation. - Optionally add a brief comment near the
return "Invalid";line explaining when that fallback is used (i.e., unknown/unhandled enum values).
These changes are all within the shown snippet, require no new imports or definitions, and preserve existing behavior. Only the lines inside the ToPrettyString function need to be updated to insert comments; no signatures, types, or logic are altered.
| @@ -1528,7 +1528,16 @@ | ||
| * @return The corresponding string. | ||
| */ | ||
| P1_CONSTEXPR_FUNC const char* ToPrettyString(GNSSSignalType type) { | ||
| // NOTE: | ||
| // - This function maps each GNSSSignalType enum value to a concise, | ||
| // human-readable label for logs, debugging output, and user interfaces. | ||
| // - The mappings here should be kept in sync with the GNSSSignalType | ||
| // definition above. When adding a new signal type, add a corresponding | ||
| // pretty-print string in the appropriate constellation section below. | ||
| switch (type) { | ||
| // | ||
| // GPS signals. | ||
| // | ||
| case GNSSSignalType::GPS_L1CA: | ||
| return "GPS C/A"; | ||
|
|
||
| @@ -1703,6 +1711,7 @@ | ||
| case GNSSSignalType::UNKNOWN: | ||
| return "Unknown"; | ||
| } | ||
| // Fallback for unknown or unsupported GNSSSignalType values. | ||
| return "Invalid"; | ||
| } | ||
|
|
wbrannon
force-pushed
the
update-signal-msg
branch
from
3eb8aea to
fab616f
Compare
New Features
GNSSSignalTypeenum definition, as well as additional signal name enums and other helper functionssignal_defs.pyfileGNSSSignalsMessagemessageChanges
GNSSSatelliteMessageis now deprecated in favor ofGNSSSignalsMessagev