Skip to content

Add embedded #12

Open
patrickelectric wants to merge 2 commits intobluerobotics:masterfrom
patrickelectric:embedded
Open

Add embedded #12
patrickelectric wants to merge 2 commits intobluerobotics:masterfrom
patrickelectric:embedded

Conversation

@patrickelectric
Copy link
Copy Markdown
Member

No description provided.

Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Comment thread guidelines/embedded.md

- Follow the project code style and keep it consistent within each file.
- Prefer small, focused functions over long procedural blocks.
- Prefer compile-time checksm add run-time checks for safety-critical paths.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
- Prefer compile-time checksm add run-time checks for safety-critical paths.
- Prefer compile-time checks, and add run-time checks only for safety-critical paths.

Comment thread guidelines/embedded.md
## Preprocessor rules

- Only use `#pragma once` and `#include`
- Do not use `#define` for inline macros or constants, use `inline`, `const`, `constexpr`, or `consteval` instead.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This assumes that C++ only code is being used, do we want to broad this for maybe Rust/C?

Comment thread guidelines/embedded.md
- No spaces around `.` or `->`, and no spaces around unary operators.
- Naming rules follow the project standard. If none is defined, use:
- `lower_snake_case` for functions and variables
- `UpperCamel_Case` for classes, structs, enums, and typedefs (e.g., `AP_Compass`)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is not a valid UpperCamelCase (PascalCase); it is a mixture of naming conventions.
Unless dictated by a specific project style, we should avoid this and try to maintain a single, consistent naming convention.
Mixing conventions or having different rules tends to reduce readability and maintainability, as seen in APIs such as OpenGL.

Comment thread guidelines/embedded.md
- `UpperCamel_Case` for classes, structs, enums, and typedefs (e.g., `AP_Compass`)
- lowercase constants and enumerators
- uppercase acronyms inside identifiers (e.g., `GCS_MAVLink`)
- use underscore for private properties
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is against AV Rule 47

Image

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we expect to use lots of libraries in our embedded code?
I suspect the compiler would catch any name collisions caused by this, and it seems potentially valuable to indicate throughout the code when something is private 🤷‍♂️

Comment thread guidelines/embedded.md
## Class design

- Use `struct` for plain data, `class` for invariants and encapsulation.
- Keep data members private, avoid public/protected data in classes.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Most of C++ references tends to use member variables/functions to address properties and methods.

Suggested change
- Keep data members private, avoid public/protected data in classes.
- Keep **member variables** private; avoid use of public or protected **member variables** in C++ classes.

Comment thread guidelines/embedded.md
## Header and implementation structure

- `.h` for headers, `.cpp` for implementations.
- Headers contain declarations only, no non-const data or function definitions.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
- Headers contain declarations only, no non-const data or function definitions.
- Headers should contain declarations only; by default, they must not contain non-const data or function definitions, except for:
- inline functions
- templates
- `constexpr` or `consteval` definitions

Comment thread guidelines/embedded.md

- `.h` for headers, `.cpp` for implementations.
- Headers contain declarations only, no non-const data or function definitions.
- Exceptions: inline functions and templates.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
- Exceptions: inline functions and templates.

Join this with suggestion above

Comment thread guidelines/embedded.md
unless the platform and toolchain explicitly support it and memory costs are measured.
- Prefer fixed-size arrays, if dynamic containers are needed, use project-approved
alternatives.
- Avoid bit fields, use plain booleans.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Accordingly to JSF AV Rules [154, 155, 156]

Suggested change
- Avoid bit fields, use plain booleans.
- Avoid using bit-fields for data packing or space optimization.
- For general state and control logic, prefer plain boolean or integer members over bit-fields.
- Bit-fields may be used only when required for hardware register mapping or protocol conformance, and shall use explicitly unsigned integral or enumeration types.
- Do not rely on implementation-defined bit-field behavior.

Comment thread guidelines/embedded.md

## Style and naming

- Avoid tabs, indent consistently (use 2 or four spaces, be consistent with the project).
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
- Avoid tabs, indent consistently (use 2 or four spaces, be consistent with the project).
- Avoid tabs; use spaces for indentation. The exact width (2 or 4 spaces) must follow the project standard and remain consistent within a file.

Comment thread guidelines/embedded.md
- Do not call virtual functions from constructors or destructors.
- Use member initializer lists for non-static members.
- Declare copy/assignment when owning pointers or nontrivial destructors.
- Base classes with virtual functions must have virtual destructors.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
- Base classes with virtual functions must have virtual destructors.
- Base classes with virtual functions must have virtual destructors, unless the class is
explicitly marked as `final`.

Comment thread guidelines/embedded.md
- No spaces around `.` or `->`, and no spaces around unary operators.
- Naming rules follow the project standard. If none is defined, use:
- `lower_snake_case` for functions and variables
- `UpperCamel_Case` for classes, structs, enums, and typedefs (e.g., `AP_Compass`)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `UpperCamel_Case` for classes, structs, enums, and typedefs (e.g., `AP_Compass`)
- `UpperCamelCase` for classes, structs, enums, and typedefs
- Acronyms can be separated by underscores (e.g. `AP_AdvancedFailsafe`)

Comment thread guidelines/embedded.md
- `UpperCamel_Case` for classes, structs, enums, and typedefs (e.g., `AP_Compass`)
- lowercase constants and enumerators
- uppercase acronyms inside identifiers (e.g., `GCS_MAVLink`)
- use underscore for private properties
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we expect to use lots of libraries in our embedded code?
I suspect the compiler would catch any name collisions caused by this, and it seems potentially valuable to indicate throughout the code when something is private 🤷‍♂️

Comment thread guidelines/embedded.md
- `UpperCamel_Case` for classes, structs, enums, and typedefs (e.g., `AP_Compass`)
- lowercase constants and enumerators
- uppercase acronyms inside identifiers (e.g., `GCS_MAVLink`)
- use underscore for private properties
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
- use underscore for private properties
- use a preceding underscore for private properties (e.g. `_internal_var`)

Not sure if this was the intent (or double preceding underscore, or one before + one after, etc), but it should be clarified.

Comment thread guidelines/embedded.md
## Documentation

- Document units in names or comments for physical quantities.
- Prefer doxygen-style comments for C++ projects.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we also have preferences for other languages?

Comment thread guidelines/embedded.md
- Prefer fixed-size arrays, if dynamic containers are needed, use project-approved
alternatives.
- Avoid bit fields, use plain booleans.
- Remove dead code, do not comment out unused code.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it acceptable/recommended to include "TODO" comments in the place of code that is not yet in use elsewhere?

Comment thread guidelines/embedded.md
alternatives.
- Avoid bit fields, use plain booleans.
- Remove dead code, do not comment out unused code.
- Prefer multiplication over division for unit conversions.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How strong is this preference / how spelled out should conversions be?
As an example, should converting from degrees to radians be angle_deg * tau / 360, or angle_deg * 0.01745329251 (which then likely requires an explanatory comment), or something else? Or are they considered equivalent because the conversion factor is likely evaluated at compile time?

Suggested change
- Prefer multiplication over division for unit conversions.
- Prefer multiplication over division for unit conversions, unless it is less clear.

Comment thread guidelines/embedded.md

## Portability and build

- Target multiple embedded platforms when feasible, avoid vendor-locked toolchains.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Target multiple embedded platforms when feasible, avoid vendor-locked toolchains.
- Target multiple embedded platforms when feasible.
- Avoid vendor-locked toolchains.

Comment thread guidelines/embedded.md

- Avoid tabs, indent consistently (use 2 or four spaces, be consistent with the project).
- Use braces for all control statements.
- Braces for blocks on their own lines and aligned.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this include starting braces, and braces that are part of block continuations?

I'm personally more inclined to

if (condition) {
    ...
} else {
    ...
}

than

if (condition)
{
    ...
}
else
{
    ...
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants