Implementing dynamic databus registry

[nielsnl68]

What does this implement/fix?

This PR enables us to dynamically add more databus drivers when needed.
It is very easy to implement.

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Other

Related issue or feature (if applicable): fixes

Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#

Test Environment

• ESP32
• ESP32 IDF
• ESP8266
• RP2040
• BK72xx
• RTL87xx

Example entry for config.yaml:

# Example config.yaml

Checklist:

• The code change is tested and works locally.
• Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated in esphome-docs.

[nielsnl68]

Okay this is what i wanted to do for the PR i was working on.
Everything compiles fine.

[clydebarrow]

Just started looking at this.

Lots of CI checks are failing.

What are all these in ili9xxx/display.py?

            "par8",
            "par9",
            "par16",
            "par18",
            "rgb16",
            "rgb18",
            "rgb24",

These obviously refer to bus names, but should be constants, not literal strings:

            "spi",
            "i80",

The values defined in ili9xxx/display.py in the MODELS dict should be either dicts or classes. References like MODELS[model][1] and MODELS[model][2][0] don't mean much to someone reading the code.

Compiling existing yamls fails with errors:

  File "/Users/clyde/dev/opensourceprojects/esp/esphome/esphome/components/ili9xxx/display.py", line 322, in give_default_bus_type
    return MODELS[model][2][0]
           ~~~~~~^^^^^^^
KeyError: 'ili9341'

def spi_device_schema(
    cs_pin_required=True,
    default_data_rate=cv.UNDEFINED,
    default_mode=cv.UNDEFINED,
    quad=False,
    dc_pin_required=True,
    use_16BitData=False,
):

dc_pin_required should default to False - spi devices other than displays don't need a DC pin.

The "16bit_data" spi type should be removed - that's not a variation on the bus, it's a variation in how data is transferred for a specific peripheral, and there is no way of knowing in advance how that should be handled.

As discussed before, there is no purpose in allowing multiple begin_transaction calls - transactions can't be nested, and any attempt to do so is an error.

  void begin_transaction() {
    if (++this->transaction_counter == 0) {
      do_begin_transaction();
    }
  }

more to come...

[nielsnl68]

Just started looking at this.

Thanks.

Lots of CI checks are failing.

Not sure where they coming from but those i checked did not seem related to my changes.

What are all these in ili9xxx/display.py?

            "par8",
            "par9",
            "par16",
            "par18",
            "rgb16",
            "rgb18",
            "rgb24",

Those are the busses the display controller can handle. This does not mean that we support them.
The different options will give the compiler the possibility to validate of the user has added enough pins for the chosen bus architecture. "par" is an alias for the i80 interface and "rgb" is an alias for the RPi_dpi interface/

These obviously refer to bus names, but should be constants, not literal strings:

I agree and i have modified those now.

            "spi",
            "i80",

The values defined in ili9xxx/display.py in the MODELS dict should be either dicts or classes. References like MODELS[model][1] and MODELS[model][2][0] don't mean much to someone reading the code.

Oww i did not read your suggestion here correct. At the moment i created some constant's defining what is what. But i think we should use the Registry solution here as well. I already implemented it in my display_driver and can implement it here as well. But can this wait until this our PR is merged?

Compiling existing yamls fails with errors:

  File "/Users/clyde/dev/opensourceprojects/esp/esphome/esphome/components/ili9xxx/display.py", line 322, in give_default_bus_type
    return MODELS[model][2][0]
           ~~~~~~^^^^^^^
KeyError: 'ili9341'

Could you show the display part of your yaml so i can see what did happen?

def spi_device_schema(
    cs_pin_required=True,
    default_data_rate=cv.UNDEFINED,
    default_mode=cv.UNDEFINED,
    quad=False,
    dc_pin_required=True,
    use_16BitData=False,
):

dc_pin_required should default to False - spi devices other than displays don't need a DC pin.

Oeps of cource that should have been false.

The "16bit_data" spi type should be removed - that's not a variation on the bus, it's a variation in how data is transferred for a specific peripheral, and there is no way of knowing in advance how that should be handled.

As discussed before, there is no purpose in allowing multiple begin_transaction calls - transactions can't be nested, and any attempt to do so is an error.

Sorry, that is your opinion. I disagree, i have used many communication packages in the past from different vendors, that worked this way.
The SPI bus does not need to be opened and closed for every command that we send. It is okay to open once and send all the commands and close the bus after that.

As i said on Discord. when we let the display driver handle the opening and closing of the transaction. and remove it in the "send_command" future then we do not need to nest here.

  void begin_transaction() {
    if (++this->transaction_counter == 0) {
      do_begin_transaction();
    }
  }

more to come...

I cant wait to see what you find more 🤞🏼

Sadly i'm not able to update this per anymore, i will send you the changes via discord.