improve port expander handling #283
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The PORT_ExpanderISR() is called very often although no interrupt was triggered by the PCA9555. The interrupt line is held low by the PCA9555 until the input registers are read. So if the interrupt line is not low, there is no need to read the registers.
The interrupt line is active low so switch from FALLING to ONLOW. To then prevent continuous calling of the ISR, it needs to be disabled. Port_ExpanderHandler() re-enables it after handling things. It also takes care to enable it in the first place. While at it, use digitalPinToInterrupt() to convert the pin to the interrupt number as recommended by the Arduino reference manual (https://www.arduino.cc/reference/en/language/functions/external-interrupts/attachinterrupt/).
Port_ExpanderHandler() was using 4 transfers to read 2 registers from the PCA9555: START-DEV_ADDR-REG_ADDR(0)-STOP START-DEV_ADDR-REG_VAL(@0)-STOP START-DEV_ADDR-REG_ADDR(1)-STOP START-DEV_ADDR-REG_VAL(@1)-STOP This takes at least 766.1 microseconds on a 100kHz bus. Simplifying this by using repeated start and reading both registers at once to: START-DEV_ADDR-REG_ADDR(0)-START-DEV_ADDR-REG_VAL(@0)-REG_VAL(@1)-STOP Now it takes at least 466.7 microseconds i.e. almost 300 microseconds less.
Instead of checking if the whole register is unchanged, check the bits independently so a toggling input doesn't prevent stable inputs from being accepted.
|
I've tested against the complete profile, it uses 108 for headphone detect. It's also working fine. 108 => port 1 channel/bit 0 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a collection of changes I made for the port expander:
prevent the PE ISR from doing anything if there was no real interrupt
I've seen the ISR being called all the time and thus Port_ExpanderHandler() accessing the bus every time it gets called. That wastes a lot of time which can be spent more useful. So I added a check in the ISR if there was really an interrupt request from the PE.
correct interrupt mode for the PE
The PCA9555 interrupt output is level low, not falling edge. That is not very different if there is only one device using the line but there is a small chance that input register 0 changes while reading input register 1. And in that case the interrupt line would stay low which won't be detected when it is set for falling edge.
So using ONLOW fixes that but requires that the interrupt is disabled in the ISR and re-enabled after reading the input registers.
speed up access to the PE
The current implementation isn't wrong, just horribly slow because to read 2 bytes from the PE it puts 3 unnecessary bytes on the bus plus i2c timing overhead.
make the debouncer work per bit
I didn't encounter an issue but if one of the bits in the input registers was changing all the time, all the other bits would be ignored. There shouldn't be a debouncer at all but until there is a thread safe i2c handling, that can't be avoided.