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

changes as necessary for multiple HATs #12

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ifurusato
Copy link

This introduces some backward-compatible changes to the Inventor HAT Mini library to permit multiple HATs co-existing. This disables audio and the User button on anything but the default I2C address of 0x17, since those pins would be in conflict.

@ZodiusInfuser
Copy link
Member

Hi @ifurusato. Thank you very much for this! I have a few minor comments I'll list separately.

Comment on lines +109 to +112
self._pin_user_sw = gpiodevice.get_pin(self.PI_USER_SW_PIN, "IHM-SW", INPD)

# Setup amplifier enable. This mutes the audio by default
self._pin_amp_en = gpiodevice.get_pin(self.PI_AMP_EN_PIN, "IHM-AMP-En", OUTL if start_muted else OUTH)
# Setup amplifier enable. This mutes the audio by default
self._pin_amp_en = gpiodevice.get_pin(self.PI_AMP_EN_PIN, "IHM-AMP-En", OUTL if start_muted else OUTH)
Copy link
Member

Choose a reason for hiding this comment

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

Since these are not assigned for the second IHM, I assume that switch_pressed(), mute_audio(), and unmute_audio() will fail if called? Can you include some handling for this? E.g. set self._pin_user_sw and self._pin_amp_en to None then check for that in all relevant places?

@coveralls
Copy link

Pull Request Test Coverage Report for Build 11788891006

Details

  • 7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.4%) to 56.604%

Totals Coverage Status
Change from base Build 9401983739: 0.4%
Covered Lines: 180
Relevant Lines: 318

💛 - Coveralls

@Gadgetoid
Copy link
Member

It's possible for multiple library instances to share the same "pin", they just can't both claim it. I did something to this end in the ST7789 library where if pre-claimed pins (basically just a gpiod lines object and an offset) are passed into the constructor, it will use them directly. Might also be useful here?

pimoroni/st7789-python@0b3cba8

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.

4 participants