[ch422g] Add support for pins 8-11; make input work. (#7467)

CODEOWNERS

@@ -86,7 +86,7 @@ esphome/components/cap1188/* @mreditor97
 esphome/components/captive_portal/* @OttoWinter
 esphome/components/ccs811/* @habbie
 esphome/components/cd74hc4067/* @asoehlke
-esphome/components/ch422g/* @jesterret
+esphome/components/ch422g/* @clydebarrow @jesterret
 esphome/components/climate/* @esphome/core
 esphome/components/climate_ir/* @glmnet
 esphome/components/color_temperature/* @jesserockz

esphome/components/ch422g/__init__.py

@@ -1,18 +1,20 @@
 from esphome import pins
 import esphome.codegen as cg
 from esphome.components import i2c
+from esphome.components.i2c import I2CBus
 import esphome.config_validation as cv
 from esphome.const import (
+    CONF_I2C_ID,
     CONF_ID,
     CONF_INPUT,
     CONF_INVERTED,
     CONF_MODE,
     CONF_NUMBER,
+    CONF_OPEN_DRAIN,
     CONF_OUTPUT,
-    CONF_RESTORE_VALUE,
 )
 
-CODEOWNERS = ["@jesterret"]
+CODEOWNERS = ["@jesterret", "@clydebarrow"]
 DEPENDENCIES = ["i2c"]
 MULTI_CONF = True
 ch422g_ns = cg.esphome_ns.namespace("ch422g")
@@ -23,29 +25,36 @@ CH422GGPIOPin = ch422g_ns.class_(
 )
 
 CONF_CH422G = "ch422g"
-CONFIG_SCHEMA = (
+
-    cv.Schema(
+# Note that no address is configurable - each register in the CH422G has a dedicated i2c address
-        {
+CONFIG_SCHEMA = cv.Schema(
-            cv.Required(CONF_ID): cv.declare_id(CH422GComponent),
+    {
-            cv.Optional(CONF_RESTORE_VALUE, default=False): cv.boolean,
+        cv.GenerateID(CONF_ID): cv.declare_id(CH422GComponent),
-        }
+        cv.GenerateID(CONF_I2C_ID): cv.use_id(I2CBus),
-    )
+    }
-    .extend(cv.COMPONENT_SCHEMA)
+).extend(cv.COMPONENT_SCHEMA)
-    .extend(i2c.i2c_device_schema(0x24))
-)
 
 
 async def to_code(config):
     var = cg.new_Pvariable(config[CONF_ID])
-    cg.add(var.set_restore_value(config[CONF_RESTORE_VALUE]))
     await cg.register_component(var, config)
-    await i2c.register_i2c_device(var, config)
+    # Can't use register_i2c_device because there is no CONF_ADDRESS
+    parent = await cg.get_variable(config[CONF_I2C_ID])
+    cg.add(var.set_i2c_bus(parent))
+
+
+# This is used as a final validation step so that modes have been fully transformed.
+def pin_mode_check(pin_config, _):
+    if pin_config[CONF_MODE][CONF_INPUT] and pin_config[CONF_NUMBER] >= 8:
+        raise cv.Invalid("CH422G only supports input on pins 0-7")
+    if pin_config[CONF_MODE][CONF_OPEN_DRAIN] and pin_config[CONF_NUMBER] < 8:
+        raise cv.Invalid("CH422G only supports open drain output on pins 8-11")
 
 
 CH422G_PIN_SCHEMA = pins.gpio_base_schema(
     CH422GGPIOPin,
-    cv.int_range(min=0, max=7),
+    cv.int_range(min=0, max=11),
-    modes=[CONF_INPUT, CONF_OUTPUT],
+    modes=[CONF_INPUT, CONF_OUTPUT, CONF_OPEN_DRAIN],
 ).extend(
     {
         cv.Required(CONF_CH422G): cv.use_id(CH422GComponent),
@@ -53,7 +62,7 @@ CH422G_PIN_SCHEMA = pins.gpio_base_schema(
 )
 
 
-@pins.PIN_SCHEMA_REGISTRY.register(CONF_CH422G, CH422G_PIN_SCHEMA)
+@pins.PIN_SCHEMA_REGISTRY.register(CONF_CH422G, CH422G_PIN_SCHEMA, pin_mode_check)
 async def ch422g_pin_to_code(config):
     var = cg.new_Pvariable(config[CONF_ID])
     parent = await cg.get_variable(config[CONF_CH422G])

esphome/components/ch422g/ch422g.cpp

@@ -4,33 +4,33 @@
 namespace esphome {
 namespace ch422g {
 
-const uint8_t CH422G_REG_IN = 0x26;
+static const uint8_t CH422G_REG_MODE = 0x24;
-const uint8_t CH422G_REG_OUT = 0x38;
+static const uint8_t CH422G_MODE_OUTPUT = 0x01;      // enables output mode on 0-7
-const uint8_t OUT_REG_DEFAULT_VAL = 0xdf;
+static const uint8_t CH422G_MODE_OPEN_DRAIN = 0x04;  // enables open drain mode on 8-11
+static const uint8_t CH422G_REG_IN = 0x26;           // read reg for input bits
+static const uint8_t CH422G_REG_OUT = 0x38;          // write reg for output bits 0-7
+static const uint8_t CH422G_REG_OUT_UPPER = 0x23;    // write reg for output bits 8-11
 
 static const char *const TAG = "ch422g";
 
 void CH422GComponent::setup() {
   ESP_LOGCONFIG(TAG, "Setting up CH422G...");
-  // Test to see if device exists
+  // set outputs before mode
-  if (!this->read_inputs_()) {
+  this->write_outputs_();
+  // Set mode and check for errors
+  if (!this->set_mode_(this->mode_value_) || !this->read_inputs_()) {
     ESP_LOGE(TAG, "CH422G not detected at 0x%02X", this->address_);
     this->mark_failed();
     return;
   }
 
-  // restore defaults over whatever got saved on last boot
+  ESP_LOGCONFIG(TAG, "Initialization complete. Warning: %d, Error: %d", this->status_has_warning(),
-  if (!this->restore_value_) {
+                this->status_has_error());
-    this->write_output_(OUT_REG_DEFAULT_VAL);
-  }
-
-  ESP_LOGD(TAG, "Initialization complete. Warning: %d, Error: %d", this->status_has_warning(),
-           this->status_has_error());
 }
 
 void CH422GComponent::loop() {
   // Clear all the previously read flags.
-  this->pin_read_cache_ = 0x00;
+  this->pin_read_flags_ = 0x00;
 }
 
 void CH422GComponent::dump_config() {
@@ -41,82 +41,99 @@ void CH422GComponent::dump_config() {
   }
 }
 
-// ch422g doesn't have any flag support (needs docs?)
+void CH422GComponent::pin_mode(uint8_t pin, gpio::Flags flags) {
-void CH422GComponent::pin_mode(uint8_t pin, gpio::Flags flags) {}
+  if (pin < 8) {
+    if (flags & gpio::FLAG_OUTPUT) {
+      this->mode_value_ |= CH422G_MODE_OUTPUT;
+    }
+  } else {
+    if (flags & gpio::FLAG_OPEN_DRAIN) {
+      this->mode_value_ |= CH422G_MODE_OPEN_DRAIN;
+    }
+  }
+}
 
 bool CH422GComponent::digital_read(uint8_t pin) {
-  if (this->pin_read_cache_ == 0 || this->pin_read_cache_ & (1 << pin)) {
+  if (this->pin_read_flags_ == 0 || this->pin_read_flags_ & (1 << pin)) {
     // Read values on first access or in case it's being read again in the same loop
     this->read_inputs_();
   }
 
-  this->pin_read_cache_ |= (1 << pin);
+  this->pin_read_flags_ |= (1 << pin);
-  return this->state_mask_ & (1 << pin);
+  return (this->input_bits_ & (1 << pin)) != 0;
 }
 
 void CH422GComponent::digital_write(uint8_t pin, bool value) {
   if (value) {
-    this->write_output_(this->state_mask_ | (1 << pin));
+    this->output_bits_ |= (1 << pin);
   } else {
-    this->write_output_(this->state_mask_ & ~(1 << pin));
+    this->output_bits_ &= ~(1 << pin);
   }
+  this->write_outputs_();
 }
 
 bool CH422GComponent::read_inputs_() {
   if (this->is_failed()) {
     return false;
   }
-
+  uint8_t result;
-  uint8_t temp = 0;
+  // reading inputs requires the chip to be in input mode, possibly temporarily.
-  if ((this->last_error_ = this->read(&temp, 1)) != esphome::i2c::ERROR_OK) {
+  if (this->mode_value_ & CH422G_MODE_OUTPUT) {
-    this->status_set_warning(str_sprintf("read_inputs_(): I2C I/O error: %d", (int) this->last_error_).c_str());
+    this->set_mode_(this->mode_value_ & ~CH422G_MODE_OUTPUT);
-    return false;
+    result = this->read_reg_(CH422G_REG_IN);
+    this->set_mode_(this->mode_value_);
+  } else {
+    result = this->read_reg_(CH422G_REG_IN);
   }
-
+  this->input_bits_ = result;
-  uint8_t output = 0;
-  if ((this->last_error_ = this->bus_->read(CH422G_REG_IN, &output, 1)) != esphome::i2c::ERROR_OK) {
-    this->status_set_warning(str_sprintf("read_inputs_(): I2C I/O error: %d", (int) this->last_error_).c_str());
-    return false;
-  }
-
-  this->state_mask_ = output;
   this->status_clear_warning();
-
   return true;
 }
 
-bool CH422GComponent::write_output_(uint8_t value) {
+// Write a register. Can't use the standard write_byte() method because there is no single pre-configured i2c address.
-  const uint8_t temp = 1;
+bool CH422GComponent::write_reg_(uint8_t reg, uint8_t value) {
-  if ((this->last_error_ = this->write(&temp, 1, false)) != esphome::i2c::ERROR_OK) {
+  auto err = this->bus_->write(reg, &value, 1);
-    this->status_set_warning(str_sprintf("write_output_(): I2C I/O error: %d", (int) this->last_error_).c_str());
+  if (err != i2c::ERROR_OK) {
+    this->status_set_warning(str_sprintf("write failed for register 0x%X, error %d", reg, err).c_str());
     return false;
   }
-
-  uint8_t write_mask = value;
-  if ((this->last_error_ = this->bus_->write(CH422G_REG_OUT, &write_mask, 1)) != esphome::i2c::ERROR_OK) {
-    this->status_set_warning(
-        str_sprintf("write_output_(): I2C I/O error: %d for write_mask: %d", (int) this->last_error_, (int) write_mask)
-            .c_str());
-    return false;
-  }
-
-  this->state_mask_ = value;
   this->status_clear_warning();
   return true;
 }
 
+uint8_t CH422GComponent::read_reg_(uint8_t reg) {
+  uint8_t value;
+  auto err = this->bus_->read(reg, &value, 1);
+  if (err != i2c::ERROR_OK) {
+    this->status_set_warning(str_sprintf("read failed for register 0x%X, error %d", reg, err).c_str());
+    return 0;
+  }
+  this->status_clear_warning();
+  return value;
+}
+
+bool CH422GComponent::set_mode_(uint8_t mode) { return this->write_reg_(CH422G_REG_MODE, mode); }
+
+bool CH422GComponent::write_outputs_() {
+  return this->write_reg_(CH422G_REG_OUT, static_cast<uint8_t>(this->output_bits_)) &&
+         this->write_reg_(CH422G_REG_OUT_UPPER, static_cast<uint8_t>(this->output_bits_ >> 8));
+}
+
 float CH422GComponent::get_setup_priority() const { return setup_priority::IO; }
 
 // Run our loop() method very early in the loop, so that we cache read values
 // before other components call our digital_read() method.
 float CH422GComponent::get_loop_priority() const { return 9.0f; }  // Just after WIFI
 
-void CH422GGPIOPin::setup() { pin_mode(flags_); }
 void CH422GGPIOPin::pin_mode(gpio::Flags flags) { this->parent_->pin_mode(this->pin_, flags); }
-bool CH422GGPIOPin::digital_read() { return this->parent_->digital_read(this->pin_) != this->inverted_; }
+bool CH422GGPIOPin::digital_read() { return this->parent_->digital_read(this->pin_) ^ this->inverted_; }
 
-void CH422GGPIOPin::digital_write(bool value) { this->parent_->digital_write(this->pin_, value != this->inverted_); }
+void CH422GGPIOPin::digital_write(bool value) { this->parent_->digital_write(this->pin_, value ^ this->inverted_); }
 std::string CH422GGPIOPin::dump_summary() const { return str_sprintf("EXIO%u via CH422G", pin_); }
+void CH422GGPIOPin::set_flags(gpio::Flags flags) {
+  flags_ = flags;
+  this->parent_->pin_mode(this->pin_, flags);
+}
 
 }  // namespace ch422g
 }  // namespace esphome

esphome/components/ch422g/ch422g.h

@@ -23,32 +23,30 @@ class CH422GComponent : public Component, public i2c::I2CDevice {
   void pin_mode(uint8_t pin, gpio::Flags flags);
 
   float get_setup_priority() const override;
-
   float get_loop_priority() const override;
-
   void dump_config() override;
 
-  void set_restore_value(bool restore_value) { this->restore_value_ = restore_value; }
-
  protected:
+  bool write_reg_(uint8_t reg, uint8_t value);
+  uint8_t read_reg_(uint8_t reg);
+  bool set_mode_(uint8_t mode);
   bool read_inputs_();
-
+  bool write_outputs_();
-  bool write_output_(uint8_t value);
 
   /// The mask to write as output state - 1 means HIGH, 0 means LOW
-  uint8_t state_mask_{0x00};
+  uint16_t output_bits_{0x00};
   /// Flags to check if read previously during this loop
-  uint8_t pin_read_cache_ = {0x00};
+  uint8_t pin_read_flags_ = {0x00};
-  /// Storage for last I2C error seen
+  /// Copy of last read values
-  esphome::i2c::ErrorCode last_error_;
+  uint8_t input_bits_ = {0x00};
-  /// Whether we want to override stored values on expander
+  /// Copy of the mode value
-  bool restore_value_{false};
+  uint8_t mode_value_{};
 };
 
-/// Helper class to expose a CH422G pin as an internal input GPIO pin.
+/// Helper class to expose a CH422G pin as a GPIO pin.
 class CH422GGPIOPin : public GPIOPin {
  public:
-  void setup() override;
+  void setup() override{};
   void pin_mode(gpio::Flags flags) override;
   bool digital_read() override;
   void digital_write(bool value) override;
@@ -57,13 +55,13 @@ class CH422GGPIOPin : public GPIOPin {
   void set_parent(CH422GComponent *parent) { parent_ = parent; }
   void set_pin(uint8_t pin) { pin_ = pin; }
   void set_inverted(bool inverted) { inverted_ = inverted; }
-  void set_flags(gpio::Flags flags) { flags_ = flags; }
+  void set_flags(gpio::Flags flags);
 
  protected:
-  CH422GComponent *parent_;
+  CH422GComponent *parent_{};
-  uint8_t pin_;
+  uint8_t pin_{};
-  bool inverted_;
+  bool inverted_{};
-  gpio::Flags flags_;
+  gpio::Flags flags_{};
 };
 
 }  // namespace ch422g

tests/components/ch422g/common.yaml

@@ -1,6 +1,5 @@
 ch422g:
   - id: ch422g_hub
-    address: 0x24
 
 binary_sensor:
   - platform: gpio
@@ -11,10 +10,18 @@ binary_sensor:
       number: 1
       mode: INPUT
       inverted: true
+output:
   - platform: gpio
-    id: ch422g_output
+    id: ch422_out_0
     pin:
       ch422g: ch422g_hub
       number: 0
       mode: OUTPUT
       inverted: false
+  - platform: gpio
+    id: ch422_out_11
+    pin:
+      ch422g: ch422g_hub
+      number: 11
+      mode: OUTPUT_OPEN_DRAIN
+      inverted: true