Mirrors_Framework/esphome
1
0
Fork 0
mirror of https://github.com/esphome/esphome.git synced 2026-05-24 18:06:27 +08:00
Code Issues Actions 8 Packages Projects Releases Wiki Activity

[axp2101] Code review fixes

Browse Source 
- Merge update() and update_sensors() methods
- Remove unnecessary variable assignments before returns
- Add #ifdef USE_SENSOR guards around sensor-related code
- Restructure number and switch to be directories for better isolation
- Use proper enum codegen helpers for PowerRail enum
This commit is contained in:
Claude
2025-11-18 19:05:21 +00:00
parent 9dba37962c
commit 8e00e895e0
8 changed files with 73 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files
esphome/components/axp2101/axp2101.cpp
+33 -33
View File
@@ -47,15 +47,39 @@ void AXP2101Component::dump_config() {
return;
}
#ifdef USE_SENSOR
// Log sensor configuration
LOG_SENSOR(" ", "Battery Voltage", this->battery_voltage_sensor_);
LOG_SENSOR(" ", "Battery Level", this->battery_level_sensor_);
LOG_SENSOR(" ", "VBUS Voltage", this->vbus_voltage_sensor_);
LOG_SENSOR(" ", "VSYS Voltage", this->vsys_voltage_sensor_);
LOG_SENSOR(" ", "Die Temperature", this->die_temperature_sensor_);
#endif
}
void AXP2101Component::update() { this->update_sensors(); }
void AXP2101Component::update() {
#ifdef USE_SENSOR
if (this->battery_voltage_sensor_ != nullptr) {
this->battery_voltage_sensor_->publish_state(this->read_battery_voltage() / 1000.0f);
}
if (this->battery_level_sensor_ != nullptr) {
this->battery_level_sensor_->publish_state(this->read_battery_level());
}
if (this->vbus_voltage_sensor_ != nullptr) {
this->vbus_voltage_sensor_->publish_state(this->read_vbus_voltage() / 1000.0f);
}
if (this->vsys_voltage_sensor_ != nullptr) {
this->vsys_voltage_sensor_->publish_state(this->read_vsys_voltage() / 1000.0f);
}
if (this->die_temperature_sensor_ != nullptr) {
this->die_temperature_sensor_->publish_state(this->read_die_temperature());
}
#endif
}
bool AXP2101Component::set_register_bit(uint8_t reg, uint8_t bit) {
uint8_t value;
@@ -312,24 +336,24 @@ uint16_t AXP2101Component::get_rail_voltage(PowerRail rail) {
}
uint16_t AXP2101Component::read_battery_voltage() {
uint16_t raw = this->read_register_h5l8(AXP2101_ADC_DATA_RELUST0, AXP2101_ADC_DATA_RELUST1);
return raw; // 1 LSB = 1mV
// 1 LSB = 1mV
return this->read_register_h5l8(AXP2101_ADC_DATA_RELUST0, AXP2101_ADC_DATA_RELUST1);
}
uint16_t AXP2101Component::read_vbus_voltage() {
uint16_t raw = this->read_register_h5l8(AXP2101_ADC_DATA_RELUST4, AXP2101_ADC_DATA_RELUST5);
return raw; // 1 LSB = 1mV
// 1 LSB = 1mV
return this->read_register_h5l8(AXP2101_ADC_DATA_RELUST4, AXP2101_ADC_DATA_RELUST5);
}
uint16_t AXP2101Component::read_vsys_voltage() {
uint16_t raw = this->read_register_h6l8(AXP2101_ADC_DATA_RELUST6, AXP2101_ADC_DATA_RELUST7);
return raw; // 1 LSB = 1mV
// 1 LSB = 1mV
return this->read_register_h6l8(AXP2101_ADC_DATA_RELUST6, AXP2101_ADC_DATA_RELUST7);
}
float AXP2101Component::read_die_temperature() {
uint16_t raw = this->read_register_h6l8(AXP2101_ADC_DATA_RELUST8, AXP2101_ADC_DATA_RELUST9);
// Temperature conversion: T = raw * 0.1 - 267.15
return (raw * TEMP_CONVERSION_FACTOR) + TEMP_OFFSET;
return (this->read_register_h6l8(AXP2101_ADC_DATA_RELUST8, AXP2101_ADC_DATA_RELUST9) * TEMP_CONVERSION_FACTOR) +
TEMP_OFFSET;
}
uint8_t AXP2101Component::read_battery_level() {
@@ -340,30 +364,6 @@ uint8_t AXP2101Component::read_battery_level() {
return level > 100 ? 100 : level;
}
void AXP2101Component::update_sensors() {
if (this->battery_voltage_sensor_ != nullptr) {
float voltage = this->read_battery_voltage() / 1000.0f;
this->battery_voltage_sensor_->publish_state(voltage);
}
if (this->battery_level_sensor_ != nullptr) {
this->battery_level_sensor_->publish_state(this->read_battery_level());
}
if (this->vbus_voltage_sensor_ != nullptr) {
float voltage = this->read_vbus_voltage() / 1000.0f;
this->vbus_voltage_sensor_->publish_state(voltage);
}
if (this->vsys_voltage_sensor_ != nullptr) {
float voltage = this->read_vsys_voltage() / 1000.0f;
this->vsys_voltage_sensor_->publish_state(voltage);
}
if (this->die_temperature_sensor_ != nullptr) {
this->die_temperature_sensor_->publish_state(this->read_die_temperature());
}
}
} // namespace axp2101
} // namespace esphome
esphome/components/axp2101/axp2101.h
+4 -3
View File
@@ -120,6 +120,7 @@ class AXP2101Component : public PollingComponent, public i2c::I2CDevice {
void update() override;
float get_setup_priority() const override { return setup_priority::HARDWARE; }
#ifdef USE_SENSOR
// Sensor setters
void set_battery_voltage_sensor(sensor::Sensor *sensor) { this->battery_voltage_sensor_ = sensor; }
void set_battery_current_sensor(sensor::Sensor *sensor) { this->battery_current_sensor_ = sensor; }
@@ -127,6 +128,7 @@ class AXP2101Component : public PollingComponent, public i2c::I2CDevice {
void set_vbus_voltage_sensor(sensor::Sensor *sensor) { this->vbus_voltage_sensor_ = sensor; }
void set_vsys_voltage_sensor(sensor::Sensor *sensor) { this->vsys_voltage_sensor_ = sensor; }
void set_die_temperature_sensor(sensor::Sensor *sensor) { this->die_temperature_sensor_ = sensor; }
#endif
// Power rail control methods
bool enable_power_rail(PowerRail rail);
@@ -144,9 +146,6 @@ class AXP2101Component : public PollingComponent, public i2c::I2CDevice {
float read_die_temperature();
uint8_t read_battery_level();
// Utility methods
void update_sensors();
protected:
// Helper methods for register access
bool set_register_bit(uint8_t reg, uint8_t bit);
@@ -161,6 +160,7 @@ class AXP2101Component : public PollingComponent, public i2c::I2CDevice {
uint8_t calculate_voltage_register_value(PowerRail rail, uint16_t millivolts);
uint16_t calculate_millivolts_from_register(PowerRail rail, uint8_t reg_value);
#ifdef USE_SENSOR
// Sensor pointers (optional)
sensor::Sensor *battery_voltage_sensor_{nullptr};
sensor::Sensor *battery_current_sensor_{nullptr};
@@ -168,6 +168,7 @@ class AXP2101Component : public PollingComponent, public i2c::I2CDevice {
sensor::Sensor *vbus_voltage_sensor_{nullptr};
sensor::Sensor *vsys_voltage_sensor_{nullptr};
sensor::Sensor *die_temperature_sensor_{nullptr};
#endif
};
} // namespace axp2101
esphome/components/axp2101/number.py → esphome/components/axp2101/number/__init__.py
+17 -16
View File
@@ -4,7 +4,7 @@ import esphome.config_validation as cv
from esphome.components import number
from esphome.const import CONF_ID, UNIT_VOLT
from . import AXP2101Component, axp2101_ns
from .. import AXP2101Component, axp2101_ns
CODEOWNERS = ["@esphome/core"]
DEPENDENCIES = ["axp2101"]
@@ -14,22 +14,23 @@ CONF_POWER_RAIL = "power_rail"
AXP2101Number = axp2101_ns.class_("AXP2101Number", number.Number, cg.Component)
# Power rail options matching the PowerRail enum in C++
# Power rail enum matching the PowerRail enum in C++
PowerRail = axp2101_ns.enum("PowerRail")
POWER_RAILS = {
"DCDC1": 0,
"DCDC2": 1,
"DCDC3": 2,
"DCDC4": 3,
"DCDC5": 4,
"ALDO1": 5,
"ALDO2": 6,
"ALDO3": 7,
"ALDO4": 8,
"BLDO1": 9,
"BLDO2": 10,
"CPUSLDO": 11,
"DLDO1": 12,
"DLDO2": 13,
"DCDC1": PowerRail.DCDC1,
"DCDC2": PowerRail.DCDC2,
"DCDC3": PowerRail.DCDC3,
"DCDC4": PowerRail.DCDC4,
"DCDC5": PowerRail.DCDC5,
"ALDO1": PowerRail.ALDO1,
"ALDO2": PowerRail.ALDO2,
"ALDO3": PowerRail.ALDO3,
"ALDO4": PowerRail.ALDO4,
"BLDO1": PowerRail.BLDO1,
"BLDO2": PowerRail.BLDO2,
"CPUSLDO": PowerRail.CPUSLDO,
"DLDO1": PowerRail.DLDO1,
"DLDO2": PowerRail.DLDO2,
}
# Voltage ranges for each power rail (in millivolts)
esphome/components/axp2101/axp2101_number.cpp → esphome/components/axp2101/number/axp2101_number.cpp
View File
esphome/components/axp2101/axp2101_number.h → esphome/components/axp2101/number/axp2101_number.h
+1 -1
View File
@@ -6,7 +6,7 @@
#include "esphome/core/component.h"
#include "esphome/components/number/number.h"
#include "axp2101.h"
#include "../axp2101.h"
namespace esphome {
namespace axp2101 {
esphome/components/axp2101/switch.py → esphome/components/axp2101/switch/__init__.py
+17 -16
View File
@@ -4,7 +4,7 @@ import esphome.config_validation as cv
from esphome.components import switch
from esphome.const import CONF_ID
from . import AXP2101Component, axp2101_ns
from .. import AXP2101Component, axp2101_ns
CODEOWNERS = ["@esphome/core"]
@@ -15,22 +15,23 @@ CONF_POWER_RAIL = "power_rail"
AXP2101Switch = axp2101_ns.class_("AXP2101Switch", switch.Switch, cg.Component)
# Power rail options matching the PowerRail enum in C++
# Power rail enum matching the PowerRail enum in C++
PowerRail = axp2101_ns.enum("PowerRail")
POWER_RAILS = {
"DCDC1": 0,
"DCDC2": 1,
"DCDC3": 2,
"DCDC4": 3,
"DCDC5": 4,
"ALDO1": 5,
"ALDO2": 6,
"ALDO3": 7,
"ALDO4": 8,
"BLDO1": 9,
"BLDO2": 10,
"CPUSLDO": 11,
"DLDO1": 12,
"DLDO2": 13,
"DCDC1": PowerRail.DCDC1,
"DCDC2": PowerRail.DCDC2,
"DCDC3": PowerRail.DCDC3,
"DCDC4": PowerRail.DCDC4,
"DCDC5": PowerRail.DCDC5,
"ALDO1": PowerRail.ALDO1,
"ALDO2": PowerRail.ALDO2,
"ALDO3": PowerRail.ALDO3,
"ALDO4": PowerRail.ALDO4,
"BLDO1": PowerRail.BLDO1,
"BLDO2": PowerRail.BLDO2,
"CPUSLDO": PowerRail.CPUSLDO,
"DLDO1": PowerRail.DLDO1,
"DLDO2": PowerRail.DLDO2,
}
CONFIG_SCHEMA = switch.switch_schema(AXP2101Switch).extend(
esphome/components/axp2101/axp2101_switch.cpp → esphome/components/axp2101/switch/axp2101_switch.cpp
View File
esphome/components/axp2101/axp2101_switch.h → esphome/components/axp2101/switch/axp2101_switch.h
+1 -1
View File
@@ -6,7 +6,7 @@
#include "esphome/core/component.h"
#include "esphome/components/switch/switch.h"
#include "axp2101.h"
#include "../axp2101.h"
namespace esphome {
namespace axp2101 {