Skip to content

Conversation

@blennster
Copy link

@blennster blennster commented Oct 25, 2025

This update allows having the pressure reading functionality without enabling the dimming functionality.

Summary by CodeRabbit

  • Refactor
    • Optimized sensor data transmission in the espresso machine controller. Temperature, pressure, and flow measurements are now aggregated before transmission, improving measurement consistency and timing precision during operation.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 25, 2025

Walkthrough

The sendSensorData method in GaggiMateController was refactored to aggregate sensor measurements before BLE transmission. Local variables for temperature, pressure, and flow metrics are now conditionally populated based on device capabilities and dimming status, then sent together in a single transmission rather than separately.

Changes

Cohort / File(s) Summary
Sensor Data Aggregation Refactor
lib/GaggiMateController/src/GaggiMateController.cpp
Refactored sendSensorData method to populate local float variables (temp, pressure, puckFlow, pumpFlow, puckResistance) conditionally based on capabilities and dimming status; changed from separate sensor and volumetric transmissions to a single aggregated BLE send call

Sequence Diagram

sequenceDiagram
    participant Caller
    participant sendSensorData as sendSensorData()
    participant DimmedPump
    participant BLE

    rect rgb(230, 245, 250)
        Note over sendSensorData: Old Flow: Separate Sends
        Caller->>sendSensorData: invoke
        sendSensorData->>BLE: sendSensorData (with zeros)
        sendSensorData->>BLE: sendVolumetricData
    end

    rect rgb(245, 250, 230)
        Note over sendSensorData: New Flow: Aggregated Send
        Caller->>sendSensorData: invoke
        sendSensorData->>sendSensorData: Initialize local vars (temp, pressure, flows)
        alt Dimming Enabled
            sendSensorData->>DimmedPump: Get measurements
            DimmedPump-->>sendSensorData: puckFlow, pumpFlow, puckResistance
        end
        sendSensorData->>sendSensorData: Populate all measurements
        sendSensorData->>BLE: sendSensorData (aggregated)
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify all conditional branches correctly populate the local variables before the final BLE send
  • Confirm temperature and pressure population logic matches device capabilities
  • Ensure no sensor data is lost in the transition from separate to aggregated transmission
  • Check DimmedPump measurement retrieval and variable assignment when dimming is active

Poem

🐰 Measurements dance in local arrays so fine,
Before aggregating, all in line,
Temp and pressure, flows take their place,
One BLE send at a measured pace!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Allow for pressure readings without dimmer" is directly aligned with the PR objectives, which explicitly state the update "allows having the pressure reading functionality without enabling the dimming functionality." The code changes support this by introducing conditional pressure handling that works independently of the dimming logic, enabling pressure readings to be populated when capabilities include pressure, regardless of dimming state. The title is concise, specific, and clearly communicates the primary change to a teammate reviewing the history.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c23773a and f596ae7.

📒 Files selected for processing (1)
  • lib/GaggiMateController/src/GaggiMateController.cpp (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
lib/GaggiMateController/src/GaggiMateController.cpp (2)
web/src/pages/ProfileEdit/ExtendedPhase.jsx (1)
  • pressure (46-46)
web/src/pages/ProfileEdit/StandardProfileForm.jsx (1)
  • pressure (205-205)
🔇 Additional comments (1)
lib/GaggiMateController/src/GaggiMateController.cpp (1)

209-226: Excellent refactoring that achieves the PR objective.

The changes successfully decouple pressure reading from dimming functionality:

  • Pressure is now read based solely on _config.capabilites.pressure (lines 214-216), regardless of dimming status
  • Dimming-specific metrics (puckFlow, pumpFlow, puckResistance) remain appropriately gated by the dimming capability check (lines 217-224)
  • The aggregation approach—collecting all measurements before a single sendSensorData call—is cleaner and more efficient than the previous separate transmissions

The logic is safe: the pressure sensor and dimmed pump casts are protected by the same capability checks used during initialization in setup().


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link

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.

1 participant