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

Added the OC pin usage of CH422G #7

Closed
wants to merge 1 commit into from

Conversation

H-sw123
Copy link
Contributor

@H-sw123 H-sw123 commented Oct 9, 2024

Verified on waveshare ESP32 S3 Touch LCD 4.3B.
Arduino IDE Serial Monitor:
image
Hardware connection:
937cb79c920192c366ddc51ffb25e8ae_compress

Added control interface for pins 8~11 of CH422G.
image

@Lzw655
Copy link
Collaborator

Lzw655 commented Oct 10, 2024

Hi @H-sw123,

Thank you very much for your contribution. I reviewed the method you implemented, and it is quite good. However, I have the following two thoughts:

  1. For common IO expanders, the OD register does not seem to be a universal hardware feature.
  2. For the OCx pins of the CH422G, would it be more appropriate to treat them as general-purpose IO for control?

Also, quite coincidentally, I just implemented such a feature as well, and it’s in another PR #6 . If you're interested, could you please help me review and test it? Thank you very much.

@H-sw123
Copy link
Contributor Author

H-sw123 commented Oct 17, 2024

Hi @Lzw655 ,
Due to other work reasons, I replied late.
Today, I tested the modified code in #6 and found that I could not set the working mode for 0-11. After I modified it, the test passed, and I found that enableOC_PushPull() and enableOC_OpenDrain() had no effect, and OC import could be directly controlled for output.
The code I verified has been synchronized to #7
If the code is not standardized, please modify it. If you need me to cooperate with other tests, I am very happy!

Copy link
Collaborator

@Lzw655 Lzw655 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These modifications are very well-structured, thank you very much for your contribution!
I have one final suggestion: considering that TestCH422G.ino is only used for CH422G, we can directly use ESP_IOExpander_CH422G, which would eliminate the need for using static_cast<ESP_IOExpander_CH422G *> for conversion.

examples/TestCH422G/TestCH422G.ino Outdated Show resolved Hide resolved
examples/TestCH422G/TestCH422G.ino Outdated Show resolved Hide resolved
examples/TestCH422G/TestCH422G.ino Outdated Show resolved Hide resolved
examples/TestCH422G/TestCH422G.ino Outdated Show resolved Hide resolved
@bondango
Copy link

Verified on waveshare ESP32 S3 Touch LCD 4.3B. Arduino IDE Serial Monitor: image Hardware connection: 937cb79c920192c366ddc51ffb25e8ae_compress

Added control interface for pins 8~11 of CH422G. image

This version worked for me, i was able to switch on and off a 12v light bulb ok via the OC outputs

@H-sw123 H-sw123 force-pushed the master branch 3 times, most recently from b1a6a2a to 5531264 Compare October 18, 2024 02:13
Lzw655 pushed a commit that referenced this pull request Oct 18, 2024
Lzw655 pushed a commit that referenced this pull request Oct 18, 2024
@Lzw655 Lzw655 closed this in #9 Oct 18, 2024
@Lzw655 Lzw655 closed this in 2ca1b78 Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants