Fix SN74HC595 doesn't use ESPHome HAL and add lint checks for it (#1188)

esphome/components/adc/adc_sensor.cpp

@@ -58,7 +58,7 @@ void ADCSensor::update() {
 }
 float ADCSensor::sample() {
 #ifdef ARDUINO_ARCH_ESP32
-  float value_v = analogRead(this->pin_) / 4095.0f;
+  float value_v = analogRead(this->pin_) / 4095.0f;  // NOLINT
   switch (this->attenuation_) {
     case ADC_0db:
       value_v *= 1.1;
@@ -80,7 +80,7 @@ float ADCSensor::sample() {
 #ifdef USE_ADC_SENSOR_VCC
   return ESP.getVcc() / 1024.0f;
 #else
-  return analogRead(this->pin_) / 1024.0f;
+  return analogRead(this->pin_) / 1024.0f;  // NOLINT
 #endif
 #endif
 }

esphome/components/max7219digit/max7219digit.cpp

@@ -101,7 +101,7 @@ void MAX7219Component::loop() {
 }
 
 void MAX7219Component::display() {
-  byte pixels[8];
+  uint8_t pixels[8];
   // Run this loop for every MAX CHIP (GRID OF 64 leds)
   // Run this routine for the rows of every chip 8x row 0 top to 7 bottom
   // Fill the pixel parameter with diplay data
@@ -205,29 +205,32 @@ void MAX7219Component::scroll_left() {
   this->stepsleft_++;
 }
 
-void MAX7219Component::send_char(byte chip, byte data) {
+void MAX7219Component::send_char(uint8_t chip, uint8_t data) {
   // get this character from PROGMEM
-  for (byte i = 0; i < 8; i++)
+  for (uint8_t i = 0; i < 8; i++)
     this->max_displaybuffer_[chip * 8 + i] = pgm_read_byte(&MAX7219_DOT_MATRIX_FONT[data][i]);
 }  // end of send_char
 
 // send one character (data) to position (chip)
 
-void MAX7219Component::send64pixels(byte chip, const byte pixels[8]) {
+void MAX7219Component::send64pixels(uint8_t chip, const uint8_t pixels[8]) {
-  for (byte col = 0; col < 8; col++) {  // RUN THIS LOOP 8 times until column is 7
+  for (uint8_t col = 0; col < 8; col++) {  // RUN THIS LOOP 8 times until column is 7
     this->enable();                        // start sending by enabling SPI
-    for (byte i = 0; i < chip; i++)     // send extra NOPs to push the pixels out to extra displays
+    for (uint8_t i = 0; i < chip; i++)     // send extra NOPs to push the pixels out to extra displays
       this->send_byte_(MAX7219_REGISTER_NOOP,
                        MAX7219_REGISTER_NOOP);  // run this loop unit the matching chip is reached
-    byte b = 0;                                 // rotate pixels 90 degrees -- set byte to 0
+    uint8_t b = 0;                              // rotate pixels 90 degrees -- set byte to 0
     if (this->orientation_ == 0) {
-      for (byte i = 0; i < 8; i++)                // run this loop 8 times for all the pixels[8] received
+      for (uint8_t i = 0; i < 8; i++) {
-        b |= bitRead(pixels[i], col) << (7 - i);  // change the column bits into row bits
+        // run this loop 8 times for all the pixels[8] received
+        b |= ((pixels[i] >> col) & 1) << (7 - i);  // change the column bits into row bits
+      }
     } else if (this->orientation_ == 1) {
       b = pixels[col];
     } else if (this->orientation_ == 2) {
-      for (byte i = 0; i < 8; i++)
+      for (uint8_t i = 0; i < 8; i++) {
-        b |= bitRead(pixels[i], 7 - col) << (7 - i);
+        b |= ((pixels[i] >> (7 - col)) << (7 - i));
+      }
     } else {
       b = pixels[7 - col];
     }
@@ -246,8 +249,8 @@ void MAX7219Component::send64pixels(byte chip, const byte pixels[8]) {
 uint8_t MAX7219Component::printdigit(const char *str) { return this->printdigit(0, str); }
 
 uint8_t MAX7219Component::printdigit(uint8_t start_pos, const char *s) {
-  byte chip;
+  uint8_t chip = start_pos;
-  for (chip = start_pos; chip < this->num_chips_ && *s; chip++)
+  for (; chip < this->num_chips_ && *s; chip++)
     send_char(chip, *s++);
   // space out rest
   while (chip < (this->num_chips_))

esphome/components/mcp3008/mcp3008.cpp

@@ -19,10 +19,10 @@ void MCP3008::dump_config() {
 }
 
 float MCP3008::read_data_(uint8_t pin) {
-  byte data_msb = 0;
+  uint8_t data_msb = 0;
-  byte data_lsb = 0;
+  uint8_t data_lsb = 0;
 
-  byte command = ((0x01 << 7) |          // start bit
+  uint8_t command = ((0x01 << 7) |          // start bit
                      ((pin & 0x07) << 4));  // channel number
 
   this->enable();

esphome/components/sn74hc595/sn74hc595.cpp

@@ -10,17 +10,17 @@ void SN74HC595Component::setup() {
   ESP_LOGCONFIG(TAG, "Setting up SN74HC595...");
 
   if (this->have_oe_pin_) {  // disable output
-    pinMode(this->oe_pin_->get_pin(), OUTPUT);
+    this->oe_pin_->pin_mode(OUTPUT);
-    digitalWrite(this->oe_pin_->get_pin(), HIGH);
+    this->oe_pin_->digital_write(true);
   }
 
   // initialize output pins
-  pinMode(this->clock_pin_->get_pin(), OUTPUT);
+  this->clock_pin_->pin_mode(OUTPUT);
-  pinMode(this->data_pin_->get_pin(), OUTPUT);
+  this->data_pin_->pin_mode(OUTPUT);
-  pinMode(this->latch_pin_->get_pin(), OUTPUT);
+  this->latch_pin_->pin_mode(OUTPUT);
-  digitalWrite(this->clock_pin_->get_pin(), LOW);
+  this->clock_pin_->digital_write(LOW);
-  digitalWrite(this->data_pin_->get_pin(), LOW);
+  this->data_pin_->digital_write(LOW);
-  digitalWrite(this->latch_pin_->get_pin(), LOW);
+  this->latch_pin_->digital_write(LOW);
 
   // send state to shift register
   this->write_gpio_();
@@ -28,26 +28,33 @@ void SN74HC595Component::setup() {
 
 void SN74HC595Component::dump_config() { ESP_LOGCONFIG(TAG, "SN74HC595:"); }
 
-bool SN74HC595Component::digital_read_(uint8_t pin) { return bitRead(this->output_bits_, pin); }
+bool SN74HC595Component::digital_read_(uint8_t pin) { return this->output_bits_ >> pin; }
 
 void SN74HC595Component::digital_write_(uint8_t pin, bool value) {
-  bitWrite(this->output_bits_, pin, value);
+  uint32_t mask = 1UL << pin;
+  this->output_bits_ &= ~mask;
+  if (value)
+    this->output_bits_ |= mask;
   this->write_gpio_();
 }
 
 bool SN74HC595Component::write_gpio_() {
   for (int i = this->sr_count_ - 1; i >= 0; i--) {
     uint8_t data = (uint8_t)(this->output_bits_ >> (8 * i) & 0xff);
-    shiftOut(this->data_pin_->get_pin(), this->clock_pin_->get_pin(), MSBFIRST, data);
+    for (int j = 0; j < 8; j++) {
+      this->data_pin_->digital_write(data & (1 << (7 - j)));
+      this->clock_pin_->digital_write(true);
+      this->clock_pin_->digital_write(false);
+    }
   }
 
   // pulse latch to activate new values
-  digitalWrite(this->latch_pin_->get_pin(), HIGH);
+  this->latch_pin_->digital_write(true);
-  digitalWrite(this->latch_pin_->get_pin(), LOW);
+  this->latch_pin_->digital_write(false);
 
   // enable output if configured
   if (this->have_oe_pin_) {
-    digitalWrite(this->oe_pin_->get_pin(), LOW);
+    this->oe_pin_->digital_write(false);
   }
 
   return true;

esphome/components/sx1509/sx1509.cpp

@@ -144,7 +144,7 @@ void SX1509Component::clock_(byte osc_source, byte osc_pin_function, byte osc_fr
   osc_source = (osc_source & 0b11) << 5;           // 2-bit value, bits 6:5
   osc_pin_function = (osc_pin_function & 1) << 4;  // 1-bit value bit 4
   osc_freq_out = (osc_freq_out & 0b1111);          // 4-bit value, bits 3:0
-  byte reg_clock = osc_source | osc_pin_function | osc_freq_out;
+  uint8_t reg_clock = osc_source | osc_pin_function | osc_freq_out;
   this->write_byte(REG_CLOCK, reg_clock);
 
   osc_divider = constrain(osc_divider, 1, 7);

esphome/components/ultrasonic/ultrasonic_sensor.cpp

@@ -17,7 +17,8 @@ void UltrasonicSensorComponent::update() {
   delayMicroseconds(this->pulse_time_us_);
   this->trigger_pin_->digital_write(false);
 
-  uint32_t time = pulseIn(this->echo_pin_->get_pin(), uint8_t(!this->echo_pin_->is_inverted()), this->timeout_us_);
+  uint32_t time = pulseIn(  // NOLINT
+      this->echo_pin_->get_pin(), uint8_t(!this->echo_pin_->is_inverted()), this->timeout_us_);
 
   ESP_LOGV(TAG, "Echo took %uµs", time);
 

script/ci-custom.py

@@ -255,6 +255,63 @@ def lint_conf_from_const_py(fname, match):
             "const.py directly.".format(highlight(name)))
 
 
+RAW_PIN_ACCESS_RE = r'^\s(pinMode|digitalWrite|digitalRead)\((.*)->get_pin\(\),\s*([^)]+).*\)'
+
+
+@lint_re_check(RAW_PIN_ACCESS_RE, include=cpp_include)
+def lint_no_raw_pin_access(fname, match):
+    func = match.group(1)
+    pin = match.group(2)
+    mode = match.group(3)
+    new_func = {
+        'pinMode': 'pin_mode',
+        'digitalWrite': 'digital_write',
+        'digitalRead': 'digital_read',
+    }[func]
+    new_code = highlight(f'{pin}->{new_func}({mode})')
+    return (f"Don't use raw {func} calls. Instead, use the `->{new_func}` function: {new_code}")
+
+
+# Functions from Arduino framework that are forbidden to use directly
+ARDUINO_FORBIDDEN = [
+    'digitalWrite', 'digitalRead', 'pinMode',
+    'shiftOut', 'shiftIn',
+    'radians', 'degrees',
+    'interrupts', 'noInterrupts',
+    'lowByte', 'highByte',
+    'bitRead', 'bitSet', 'bitClear', 'bitWrite',
+    'bit', 'analogRead', 'analogWrite',
+    'pulseIn', 'pulseInLong',
+    'tone',
+]
+ARDUINO_FORBIDDEN_RE = r'[^\w\d](' + r'|'.join(ARDUINO_FORBIDDEN) + r')\(.*'
+
+
+@lint_re_check(ARDUINO_FORBIDDEN_RE, include=cpp_include, exclude=[
+    'esphome/components/mqtt/custom_mqtt_device.h',
+    'esphome/core/esphal.*',
+])
+def lint_no_arduino_framework_functions(fname, match):
+    nolint = highlight("// NOLINT")
+    return (
+        f"The function {highlight(match.group(1))} from the Arduino framework is forbidden to be "
+        f"used directly in the ESPHome codebase. Please use ESPHome's abstractions and equivalent "
+        f"C++ instead.\n"
+        f"\n"
+        f"(If the function is strictly necessary, please add `{nolint}` to the end of the line)"
+    )
+
+
+@lint_re_check(r'[^\w\d]byte\s+[\w\d]+\s*=.*', include=cpp_include, exclude={
+    'esphome/components/tuya/tuya.h',
+})
+def lint_no_byte_datatype(fname, match):
+    return (
+        f"The datatype {highlight('byte')} is not allowed to be used in ESPHome. "
+        f"Please use {highlight('uint8_t')} instead."
+    )
+
+
 @lint_post_check
 def lint_constants_usage():
     errors = []