-
Notifications
You must be signed in to change notification settings - Fork 15.1k
copilot: Review instructions for uorb msgs #26169
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
Open
hamishwillee
wants to merge
6
commits into
PX4:main
Choose a base branch
from
hamishwillee:uorb_review_instructions
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
708889a
copilot: Review instructions for uorb msgs
hamishwillee 864b39e
Update .github/instructions/msg.instructions.md
hamishwillee 8f90f06
Update .github/instructions/msg.instructions.md
hamishwillee ce9433d
Update .github/instructions/msg.instructions.md
hamishwillee ed29778
Update .github/instructions/msg.instructions.md
hamishwillee d373c75
Update .github/instructions/msg.instructions.md
hamishwillee File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| --- | ||
| applyTo: "msg/**.msg" | ||
| --- | ||
|
|
||
| # Review Guidelines for PX4 uORB Message Definitions | ||
|
|
||
| You are an expert embedded software engineer specializing in the PX4 Autopilot system. | ||
| Your task is to review `.msg` files to ensure they comply with the official [PX4 uORB Documentation Standard](../../docs/en/uorb/uorb_documentation). | ||
|
|
||
| ## 1. File Structure & Header | ||
|
|
||
| - **Mandatory Short Description:** Every message must start with a `#` comment providing a succinct explanation (e.g., `# Validated airspeed`). | ||
| - No terminating full stop for single-line descriptions. | ||
| - **Long Description:** Optional but encouraged for complex messages. Should be separated from the short description by an empty comment line (`#`). Use full punctuation for multi-line descriptions. | ||
| - **Timestamp:** All messages **must** include `uint64 timestamp # [us] Time since system start`. | ||
| - **Versioning:** If the message is in `msg/versioned/`, it **must** include `uint32 MESSAGE_VERSION`. | ||
|
|
||
| ## 2. Field Documentation Standards | ||
|
|
||
| - **Inline Comments:** Fields should have a comment on the same line, separated by exactly one space from the field name. This should be followed first by metadata tags (described below) if required, and then descriptive text. | ||
| - **Metadata Tags:** | ||
| - Metadata tags appear first in the field comments, and are delineated by brackets. | ||
| - The type of metdata is indicated by a value following the @ symbol, except for unit metadata, which is assumed to be units if there is no value immediately following the @ symbol. | ||
| - The metadata types are: | ||
| - **Units:** Required for all non-boolean/non-enum fields. | ||
| - Format: `[unit]` (e.g., `[m/s]`, `[rad/s]`, `[m/s^2]`, `[deg]`). | ||
| - Unitless fields must use `[-]`. | ||
| - **Ranges:** Use `[@range min, max]` to define valid bounds. | ||
| Values for either `min` and `max` may be omitted if there is no lower or upper bound. | ||
| - **Enums:** Use `[@enum NAME]` to link a field to a set of constants defined in the file. | ||
| - The `NAME` should be the prefix of a set of constants defined in the file. | ||
| - **Invalid Values:** Use `[@invalid NaN]` to indicate that a `NaN` value represents "no data". | ||
| If another value represents "no data" that would replace `NaN` as the invalid value. A short description can follow the value by one space. | ||
| - **Field descriptions:** Field descriptions come after any metadata, if present. | ||
| - The description should start with a capital letter. If the description is a single sentence it should omit the final full stop. | ||
| - The description may contain unit or other metadata, but this metadata must also be captured in the metadata tags. | ||
| - **Example**. | ||
| The following example shows a field of type `float32` with a name `true_airspeed`. | ||
| The comment starts one space after the field name, has `unit` metadata and `invalid` metadata, followed by the descriptive text `True airspeed (TAS)`. | ||
|
|
||
| ``` | ||
| float32 true_airspeed # [m/s] [@invalid NaN] True airspeed (TAS) | ||
| ``` | ||
|
|
||
| ## 3. Constants & Enums | ||
|
|
||
| - **Naming:** Constants related to a specific field should share a common prefix in their name that matches the associated field (except in terms of case) | ||
| - **Position in message:** Constants should appear immediately after the field in which they are used, ordered by their value. | ||
| - **Documentation:** | ||
| - Constants may have a comment, separated by exactly one space from the contant name. | ||
| - The comment may contain a description of the purpose of the constant. The description should start with a capital letter. If the description is a single sentence it should omit the final full stop. | ||
| - The description/comment is optional if the purpose of the constant is obvious from its name. | ||
| - **Standard Constants:** Standardized fields like `MESSAGE_VERSION` or `ORB_QUEUE_LENGTH` do not require individual documentation. | ||
hamishwillee marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| - **Example:** The following code shows a field airspeed_source followed by the constants that can be used with it. | ||
| These all share the prefix `AIRSPEED_SOURCE` matching the field name, and this is listed in the field metadata as the enum name. | ||
| The contants are listed immediately after the field, and ordered by their value in descending order. | ||
|
|
||
| ``` | ||
| int8 airspeed_source # [@enum AIRSPEED_SOURCE] Source of currently published airspeed values | ||
| int8 AIRSPEED_SOURCE_DISABLED = -1 # Disabled | ||
| int8 AIRSPEED_SOURCE_GROUND_MINUS_WIND = 0 # Ground speed minus wind | ||
| int8 AIRSPEED_SOURCE_SENSOR_1 = 1 # Sensor 1 | ||
| int8 AIRSPEED_SOURCE_SENSOR_2 = 2 # Sensor 2 | ||
| int8 AIRSPEED_SOURCE_SENSOR_3 = 3 # Sensor 3 | ||
| int8 AIRSPEED_SOURCE_SYNTHETIC = 4 # Synthetic airspeed | ||
| ``` | ||
|
|
||
| ## 4. Multi-Topic Messages | ||
|
|
||
| - If a single `.msg` file defines multiple topics (e.g., `actuator_controls_0`, `actuator_controls_1`), the file must end with a `# TOPICS` line followed by the space-separated topic names. | ||
|
|
||
| ## 5. Field Changes | ||
|
|
||
| - For files in `./msg/versioned/` or one of its subfolders: | ||
| - The file must have a field named `MESSAGE_VERSION`. | ||
| - If a field name or type changes, or if a field is deleted or added, the value of the `MESSAGE_VERSION` field must be incremented by one. | ||
|
|
||
| ## Review Checklist | ||
|
|
||
| 1. Does it have a mandatory short description at the top? | ||
| 2. Is the `uint64 timestamp` field present? | ||
| 3. Do all numeric fields have units in square brackets? | ||
| 4. Are enums linked to fields using the `[@enum Name]` tag? | ||
| 5. Are single-line comments lacking a trailing period? | ||
| 6. If the message is versioned, is `MESSAGE_VERSION` included? | ||
hamishwillee marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Do we want to specify a naming convention for the fields themselves too?