Feature/update fan speeds#30
Conversation
This commit introduces a multi-speed fan control feature using Button A (WIO_KEY_A).
The fan can now be cycled through four states: Off, Low, Medium, and High.
Changes include:
- Added a new state variable `current_fan_speed_level` to track fan speed.
- Defined PWM constants for Off (0), Low (20), Medium (100), and High (200) speeds.
- Modified `setup()` to initialize the fan to Off using `analogWrite(D0, FAN_PWM_OFF)`.
- Updated `ButtonEventHandler` to handle `WIO_KEY_A` presses:
- Cycles through Off -> Low -> Medium -> High -> Off states.
- Uses `analogWrite(D0, ...)` with the respective PWM values.
- Added `WIO_KEY_A` to the `ButtonId` enum and initialized it in `ButtonInit`.
- Commented out the previous simple toggle fan logic associated with Button C to avoid conflicts.
version 8.0.0 not supported anymore
The PWM values for the fan speed control have been updated based on your feedback to provide a different speed curve. - FAN_PWM_LOW changed from 20 to 100 - FAN_PWM_MEDIUM changed from 100 to 150 - FAN_PWM_HIGH remains at 200 - FAN_PWM_OFF remains at 0 Button A continues to cycle through Off, Low, Medium, and High fan speed states using these new PWM values.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @directvox, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello team, gemini-code-assist here with a summary of this pull request. This PR, authored by directvox, introduces functionality to control fan speed using Pulse Width Modulation (PWM) via Button A. The previous simple ON/OFF toggle logic on Button C has been commented out in favor of this new, more granular control method. The changes also include an update to a platform dependency version in the project configuration.
Highlights
- Variable Fan Speed Control: Implements PWM-based fan speed control on pin D0, defining four levels: Off, Low, Medium, and High.
- Button A Mapping: Maps Button A to cycle through the defined fan speed levels (Off -> Low -> Medium -> High -> Off) upon release.
- Button C Logic Update: The previous ON/OFF toggle logic for the fan on Button C has been commented out, shifting the primary fan control to Button A.
- Dependency Update: Updates the
atmelsamplatform version in the PlatformIO configuration file.
Changelog
- firmware/platformio.ini
- Updated the
atmelsamplatform version from 8.0.0 to 8.3.0 (L12). - Removed a trailing newline character at the end of the file (L51).
- Updated the
- firmware/src/main.cpp
- Commented out the old
fan_statevariable and introducedcurrent_fan_speed_level(uint8_t) and constants for PWM values (FAN_PWM_OFF,LOW,MEDIUM,HIGH) to manage variable fan speeds (L288-293). - Added
ButtonId::Ato theButtonIdenumeration (L302). - Modified the
ButtonEventHandlerto handle thekEventReleasedevent forButtonId::A, implementing the logic to cycle through fan speed levels usinganalogWrite(L350-364). - Commented out the previous fan toggle logic associated with
ButtonId::C(L369-374). - Added initialization for Button A in the
ButtonInitfunction (L407-408). - Changed the initial fan state setup in
setup()to useanalogWrite(D0, FAN_PWM_OFF)instead of the olddigitalWritemethod (L466).
- Commented out the old
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Button A is pressed,
Fan speed cycles, low to high,
Cool breeze starts to blow.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request successfully implements the feature to control the fan speed using Button A, cycling through Off, Low, Medium, and High settings. The changes involve updating the platform version, adding new constants for PWM values, introducing a state variable for the fan speed level, and modifying the button event handler to manage the fan speed based on Button A presses. The old fan toggle logic on Button C has been commented out, which is a good temporary measure to avoid conflicts.
The use of constants for the PWM values is a good practice for readability and maintainability. The logic for cycling through the speeds appears correct. Overall, the core functionality requested in the PR description is implemented.
Summary of Findings
- Fan speed control logic verbosity: The
if/else if/elsestructure for cycling through fan speeds is functional but could be simplified using an array and modulo for better maintainability and extensibility. - Hardcoded fan pin: The pin
D0used for fan control is hardcoded in multiple places. Defining a constant for this pin would improve maintainability. - Commented-out code: The old fan toggle logic for Button C is commented out. While this is fine as a temporary measure, it should ideally be removed in a subsequent cleanup PR.
- Trailing newline in platformio.ini: A trailing newline was removed from the
platformio.inifile. This is a minor stylistic change.
Merge Readiness
The pull request implements the core functionality as described and introduces several positive changes like using constants for PWM values. There are a couple of medium-severity maintainability issues related to the fan speed control logic structure and the hardcoded pin number. I recommend addressing these suggestions to improve the code's maintainability before merging. I am unable to approve this pull request; please have other reviewers approve this code before merging.
| if (static_cast<ButtonId>(id) == ButtonId::A) { | ||
| if (current_fan_speed_level == 0) { // Currently Off | ||
| current_fan_speed_level = 1; // Set to Low | ||
| analogWrite(D0, FAN_PWM_LOW); | ||
| } else if (current_fan_speed_level == 1) { // Currently Low | ||
| current_fan_speed_level = 2; // Set to Medium | ||
| analogWrite(D0, FAN_PWM_MEDIUM); | ||
| } else if (current_fan_speed_level == 2) { // Currently Medium | ||
| current_fan_speed_level = 3; // Set to High | ||
| analogWrite(D0, FAN_PWM_HIGH); | ||
| } else { // Currently High (or any other state, cycle back to Off) | ||
| current_fan_speed_level = 0; // Set to Off | ||
| analogWrite(D0, FAN_PWM_OFF); | ||
| } |
There was a problem hiding this comment.
The logic for cycling through the fan speed levels using a series of if/else if/else statements is functional but can become verbose if more speed levels are added. Consider using an array of PWM values and a modulo operator to cycle through the levels, which can make the code more concise and easier to extend.
| if (static_cast<ButtonId>(id) == ButtonId::A) { | |
| if (current_fan_speed_level == 0) { // Currently Off | |
| current_fan_speed_level = 1; // Set to Low | |
| analogWrite(D0, FAN_PWM_LOW); | |
| } else if (current_fan_speed_level == 1) { // Currently Low | |
| current_fan_speed_level = 2; // Set to Medium | |
| analogWrite(D0, FAN_PWM_MEDIUM); | |
| } else if (current_fan_speed_level == 2) { // Currently Medium | |
| current_fan_speed_level = 3; // Set to High | |
| analogWrite(D0, FAN_PWM_HIGH); | |
| } else { // Currently High (or any other state, cycle back to Off) | |
| current_fan_speed_level = 0; // Set to Off | |
| analogWrite(D0, FAN_PWM_OFF); | |
| } | |
| if (static_cast<ButtonId>(id) == ButtonId::A) { | |
| const int fan_pwm_levels[] = {FAN_PWM_OFF, FAN_PWM_LOW, FAN_PWM_MEDIUM, FAN_PWM_HIGH}; | |
| const int num_levels = sizeof(fan_pwm_levels) / sizeof(fan_pwm_levels[0]); | |
| current_fan_speed_level = (current_fan_speed_level + 1) % num_levels; | |
| analogWrite(D0, fan_pwm_levels[current_fan_speed_level]); | |
| } | |
| // Handle other buttons |
| analogWrite(D0, FAN_PWM_LOW); | ||
| } else if (current_fan_speed_level == 1) { // Currently Low | ||
| current_fan_speed_level = 2; // Set to Medium | ||
| analogWrite(D0, FAN_PWM_MEDIUM); | ||
| } else if (current_fan_speed_level == 2) { // Currently Medium | ||
| current_fan_speed_level = 3; // Set to High | ||
| analogWrite(D0, FAN_PWM_HIGH); | ||
| } else { // Currently High (or any other state, cycle back to Off) | ||
| current_fan_speed_level = 0; // Set to Off | ||
| analogWrite(D0, FAN_PWM_OFF); |
There was a problem hiding this comment.
The pin D0 is hardcoded here. While common in small embedded projects, defining a constant for the fan control pin would improve maintainability and make it easier to change the pin if needed in the future.
| analogWrite(D0, FAN_PWM_LOW); | |
| } else if (current_fan_speed_level == 1) { // Currently Low | |
| current_fan_speed_level = 2; // Set to Medium | |
| analogWrite(D0, FAN_PWM_MEDIUM); | |
| } else if (current_fan_speed_level == 2) { // Currently Medium | |
| current_fan_speed_level = 3; // Set to High | |
| analogWrite(D0, FAN_PWM_HIGH); | |
| } else { // Currently High (or any other state, cycle back to Off) | |
| current_fan_speed_level = 0; // Set to Off | |
| analogWrite(D0, FAN_PWM_OFF); | |
| analogWrite(FAN_PIN, FAN_PWM_LOW); | |
| } else if (current_fan_speed_level == 1) { // Currently Low | |
| current_fan_speed_level = 2; // Set to Medium | |
| analogWrite(FAN_PIN, FAN_PWM_MEDIUM); | |
| } else if (current_fan_speed_level == 2) { // Currently Medium | |
| current_fan_speed_level = 3; // Set to High | |
| analogWrite(FAN_PIN, FAN_PWM_HIGH); | |
| } else { // Currently High (or any other state, cycle back to Off) | |
| current_fan_speed_level = 0; // Set to Off | |
| analogWrite(FAN_PIN, FAN_PWM_OFF); |
|
|
||
| pinMode(D0, OUTPUT); | ||
| digitalWrite(D0, INITIAL_FAN_STATE); | ||
| analogWrite(D0, FAN_PWM_OFF); |
There was a problem hiding this comment.
Similar to the previous comment, the pin D0 is hardcoded here. Using a constant for the fan pin would be beneficial for maintainability.
| analogWrite(D0, FAN_PWM_OFF); | |
| analogWrite(FAN_PIN, FAN_PWM_OFF); |
Adds the capability to send sensor data collected in TRAINING mode to the Edge Impulse ingestion API. Key features and changes: - Data is sent in batches (default 100 readings). - Each reading is collected at a 100ms interval. - Payloads are signed using HMAC SHA256. - Edge Impulse API Key is configured via `Config.h` (`EDGE_IMPULSE_API_KEY`). - Edge Impulse HMAC Secret Key is configurable via the serial menu (`set_ei_hmackey`) and stored in flash. - Other Edge Impulse settings (Device ID, Device Type, Label, Host, Path, Batch Size, Interval) are defined in `Config.h`. - JSON payload adheres to the Edge Impulse data ingestion format, including `protected` header with `iat` and `signature`. - Uses `HTTPClient` for secure POST requests to the Edge Impulse API. - Includes error handling and logging for the ingestion process. - New utility `CryptoUtils` for HMAC SHA256 computation using mbedTLS. - Updates to `Storage` and `ConfigurationMode` to support HMAC key management.
Corrects a compilation error in `processAndSendEdgeImpulseBatch` where `String(iat_timestamp)` was ambiguous because `iat_timestamp` is a `uint64_t`. Changed the code to explicitly cast `iat_timestamp` to `unsigned long` when constructing the filename string: `String((unsigned long)iat_timestamp)` This resolves the compiler ambiguity.
1 participant
Trigger fan speed changes by triggering button A