angelnu / esphome

Settings for esphome
Apache License 2.0
6 stars 2 forks source link

Shelly Pro4PM status? #3

Open pihiandreas opened 4 weeks ago

pihiandreas commented 4 weeks ago

Hi angelnu, The work you've done on the esphome conf for Shelly Pro4PM look really promising, but I'm sure not I've found the most recent version. What is the status of this work? I've got the devices/shelly/pro4pm/base_v1.yaml installed on a Pro4PM, but the ade7953-readings all show rubbish and only switch 4 triggers the proper relay. Do I need to use some special version of spi ? Cheers!

pihiandreas commented 4 weeks ago

Update on this issue. While debugging I've found that the issue with the Pro4PM's 4 switches is related to esp32-framework used. It works if I stick to framework: arduino (as you do) but I want to use framework: esp-idf. For some reason the GPIO-extender (mcp23s17) doesn't work properly over esp-idf-version of the generic spi-component.

angelnu commented 4 weeks ago

I just did a push of my latest config. This is what I am curre tly using. Indeed had to switch to arduino as framework but ido not recall the reason...

pihiandreas commented 4 weeks ago

Thanks for the update. It looks like most of my problems relate to the SPI-implementation for esp-idf. With some modification of your yaml for the Pro4PM it seems to work well (using arduino-framework), after all. I opened an issue at esphome, hope to get some help there. Pls consider this issue resolved and thanks again for the good work.

angelnu commented 4 weeks ago

Good to hear. I would be curious about what your changes are as I just got the basic features to work but I am still not convinced about the display code: it takes too long to refresh which results in warnings.

pihiandreas commented 2 weeks ago

OMG this is a beast ... roughly 200 build&flash iterations later .. yeah I'm still learning .. I've reached the following conclusions:

The code is available here:

external_components:
  - source:
      type: git
      url: https://github.com/andreas-berg/esphome_sync_ab.git
      ref: idf-spi-dev
    components: [spi, ade7953_spi, ade7953_base, mcp23s17]

The ade7953_spi is stable and should work on both esp-idf and arduino. I did some refactoring of the ade7953_base for my own personal use, feel free to adopt it into the official version if you find it useful, the ade7953_i2c is also updated accordingly (and tested on Shelly Plus2PM). Also implemented the same spi-changes to the mcp23s17 but this is not stable. The Shelly Pro4PM relays work only after hard-reboot / power-off. And I gave up with the switches since they require setting up and listening to proper IRQs from over spi which would require even more work on the components/spi.

Also, I don't understand how the 3 devices on the same spi_bus (shared CLK/MISO/MOSI, separate CS) can possibly be stable when in this case the 2 ade7953:s are set at clock_speed 1MHz (max 5MHz) and the mcp23s17 is set at 8MHz, maybe the arduino-framework does some kind of arbitration (ie adjusts to the slowest) but I don't think esp-idf does (or even can do) this.

Never got as far as to properly testing the display, the basic st7735-display works (also on esp-idf) according to your yaml and I only get the "normal" Component xyz took a long time for an operation (xx ms)-warning. I say "normal" because the long runner-issue does not look like it's going to be resolved (by the core team).

For reference I restructured the update-loop to separate get-data- (multiple reads from ade7953 over spi or i2c) and publish-data-functions. On a Shelly Plus2PM (i2c, 400kHz), with 14 sensor-values updated, the logs are as follows:

[D][ade7953:310]: [3761734318] Update loop done in            136.931 ms
[D][ade7953:311]: [3761734318]   Get data 5.309 ms,   publish 131.622 ms
[W][component:237]: Component ade7953_base took a long time for an operation (149 ms).
[W][component:238]: Components should block for at most 30 ms.

In general, I don't think there much to be improved in the component-code but someone should pay more attention to improving the publish_state().

angelnu commented 2 weeks ago

Quick reply while on the go:

Would you consider contributing your spi rework? If it passes the testcases it would improve the situation with all tre wrappers...