Skip to content

2 core foundation error handling - #15

Merged
lxsaah merged 5 commits into
mainfrom
2-core-foundation-error-handling
Sep 24, 2025
Merged

2 core foundation error handling#15
lxsaah merged 5 commits into
mainfrom
2-core-foundation-error-handling

Conversation

@lxsaah

@lxsaah lxsaah commented Sep 24, 2025

Copy link
Copy Markdown
Contributor

Adds the design document for milestone 1 of issue #2.

Milestone 2 is still open!

Copilot AI left a comment

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.

Pull Request Overview

Adds a comprehensive design document for milestone 1 of issue #2, establishing the foundation for AimDB's error handling and feature flag architecture. The design addresses cross-platform error handling requirements from MCU to cloud environments with unified API surfaces.

  • Defines unified DbError type with platform-conditional behavior using feature flags
  • Establishes feature flag architecture for std/embedded platforms with runtime and protocol extensions
  • Creates structured error taxonomy with performance constraints and observability integration

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
docs/design/000-M1_error-feature-flags.md Complete design document defining error handling architecture, feature flags, and cross-platform abstractions for M1 milestone
.github/chatmodes/architecture.chatmode.md Removes project planning sections to focus purely on architecture and design patterns

Comment thread docs/design/000-M1_error-feature-flags.md Outdated
Comment thread docs/design/000-M1_error-feature-flags.md Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@lxsaah
lxsaah requested a review from Copilot September 24, 2025 20:18

Copilot AI left a comment

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.

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

Comment thread docs/design/000-M1_error-feature-flags.md
Comment thread docs/design/000-M1_error-feature-flags.md
Comment thread docs/design/000-M1_error-feature-flags.md
Comment thread docs/design/000-M1_error-feature-flags.md
@lxsaah
lxsaah requested a review from Copilot September 24, 2025 20:24

Copilot AI left a comment

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.

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

pub use crate::error::StdErrorExt;
}

#[cfg(all(not(feature = "std"), feature = "embedded"))]

Copilot AI Sep 24, 2025

Copy link

Choose a reason for hiding this comment

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

The condition all(not(feature = "std"), feature = "embedded") may be redundant since the design states that embedded and std are mutually exclusive. Consider simplifying to just #[cfg(feature = "embedded")] if the mutual exclusion is enforced elsewhere, or document why the explicit check is necessary.

Suggested change
#[cfg(all(not(feature = "std"), feature = "embedded"))]
#[cfg(feature = "embedded")]

Copilot uses AI. Check for mistakes.
Comment on lines +198 to +200
❌ mqtt + embedded # Network stack not available
```

Copilot AI Sep 24, 2025

Copy link

Choose a reason for hiding this comment

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

The invalid combinations list shows mqtt + embedded as invalid, but this conflicts with the IoT use case where MCUs commonly use MQTT. Consider clarifying whether this means 'MQTT over TCP is not available on embedded' or if there should be a separate mqtt-embedded feature for lightweight MQTT implementations.

Suggested change
❌ mqtt + embedded # Network stack not available
```
❌ mqtt + embedded # MQTT over TCP not available on embedded; use `mqtt-embedded` for lightweight/MCU MQTT (if available)

Note: The combination mqtt + embedded is invalid because the standard MQTT implementation requires a TCP/IP stack, which is typically unavailable or unsuitable on MCU/embedded platforms. If a lightweight MQTT implementation (e.g., over custom transport or minimal TCP) is supported, use the mqtt-embedded feature instead.

Copilot uses AI. Check for mistakes.
### Architectural Constraints

**Memory Constraints (Embedded)**
- Error enum size ≤ 64 bytes (single cache line)

Copilot AI Sep 24, 2025

Copy link

Choose a reason for hiding this comment

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

The 64-byte constraint assumes a specific cache line size, but MCUs often have different cache architectures or no cache at all. Consider specifying this as a general memory efficiency constraint rather than tying it to cache line size, or provide platform-specific size targets.

Suggested change
- Error enum size ≤ 64 bytes (single cache line)
- Error enum size ≤ 64 bytes (desktop/edge); ≤ 32 bytes recommended for MCUs. Target minimal size for memory efficiency.

Copilot uses AI. Check for mistakes.
@lxsaah
lxsaah merged commit 34e8e1d into main Sep 24, 2025
2 checks passed
@lxsaah
lxsaah deleted the 2-core-foundation-error-handling branch September 24, 2025 20:26
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.

2 participants