Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support spi for sn74hc595 #5491

Merged
merged 17 commits into from
Nov 7, 2023
Merged

support spi for sn74hc595 #5491

merged 17 commits into from
Nov 7, 2023

Conversation

angelnu
Copy link
Contributor

@angelnu angelnu commented Oct 7, 2023

What does this implement/fix?

Support using SPI for the sn74hc595. This is required when other SPI devices are in the same bus such as by the Shelly Pro 2PM.

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): NA

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

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040

Example entry for config.yaml:

When using the SPI the following can be used:

# Example with SPI
spi:
  clk_pin: GPIO21
  mosi_pin: GPIO22
  miso_pin: GPIO23
sn74hc595:
  - id: sn74hc595_hub_2
    latch_pin: GPIO22
    oe_pin: GPIO32
    sr_count: 2

When not using the SPI, the same previous schema works:

sn74hc595:
  - id: sn74hc595_hub
    data_pin: GPIO21
    clock_pin: GPIO23
    latch_pin: GPIO22
    oe_pin: GPIO32
    sr_count: 2

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:

@angelnu
Copy link
Contributor Author

angelnu commented Oct 25, 2023

@notsonominal Did this one also work gor you?

@ghost
Copy link

ghost commented Oct 26, 2023

Works great both as SPI and non-SPI on Shelly pro and Shelly plus models I tested.

@jesserockz
Copy link
Member

I just made some (untested) changes to the code to clean it up a bit and truly split spi vs gpio right from codegen.

Please make sure they work for you and let me know

@angelnu
Copy link
Contributor Author

angelnu commented Oct 27, 2023

I can confirm it compiles for me. Nice rework.

@notsonominal could you please retest on your side? I do not have this device.

@ghost
Copy link

ghost commented Nov 6, 2023

yeah, still works both as spi and no spi 👍

@kbx81 kbx81 merged commit ccffbfd into esphome:dev Nov 7, 2023
46 of 47 checks passed
@ghost
Copy link

ghost commented Nov 7, 2023

This might have been premature as i compiled and tested 228c258 while 92b618c was merged.
It works when checking out 2023.11.0-dev
but moving from spi to not spi (with no spi bus configured) i get this

Compiling .pioenvs/shelly-pro-1pm-30c6f781837c/src/esphome/components/socket/socket.o
*** [.pioenvs/shelly-pro-1pm-30c6f781837c/src/esphome/components/spi/spi.o] Source `src/esphome/components/spi/spi.cpp' not found, needed by target `.pioenvs/shelly-pro-1pm-30c6f781837c/src/esphome/components/spi/spi.o'.
========================================= [FAILED] Took 12.09 seconds =========================================

clean and run gets past it though, and both still works

@angelnu
Copy link
Contributor Author

angelnu commented Nov 7, 2023

I can take a look at the warning this evening. Can you dm your config (just in case)?

@angelnu
Copy link
Contributor Author

angelnu commented Nov 7, 2023

@notsonominal - I am not able to reproduce the warning you are getting. So could you open an issue and attach your configuration? It might be depending on the framework/device...

@jesserockz jesserockz mentioned this pull request Nov 8, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Nov 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants