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

improve port expander handling #282

Closed
wants to merge 5 commits into from
Closed

Conversation

r-schmidt
Copy link
Contributor

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 (no idea where those spurious interrupts come from)
  • speed up access to the PE
  • make the debouncer work per bit

The output register status buffer was initialized fix to {0xff,0xff}
although the output registers were set to {0x00,0x00}.
That buffer is used by Port_Write() to determine the values of the
bits for the other pins. So those get set to 1 (high).

Since the peripheral power on the Espuino 4-layer mini PCB is switched
on by setting a pin on the PCA9555 low, that power is first turned on
(by Port_Init()), then turned off (by the Port_Write() call in
AudioPlayer_Init()) and then turned on again (by Power_PeripheralOn()).
That caused a 12.5 millisecond disruption of the external 3.3V which
some SD-cards don't like.
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.
@tueddy
Copy link
Collaborator

tueddy commented Dec 26, 2023

@r-schmidt Thank you very much for your contribution!

Please do the PR in the DEV branch first if possible, we can still test the extensions intensively there.
Part of your extension is already included in the DEV branch, see here:
2ef1b07

As discussed we use uint_8 not unsigned int in loop:
https://forum.espuino.de/t/neustart-schlaegt-fehl-abhaengig-von-sd-karte/2460/31

Where exactly did you encounter problems with the current implementation? Perhaps you can briefly describe it in the forum so that everyone can get an idea of how you have done speedup & debounce

@r-schmidt
Copy link
Contributor Author

Seems like I need to make a new PR to change the base branch on my side.

@r-schmidt r-schmidt closed this Dec 26, 2023
@tueddy
Copy link
Collaborator

tueddy commented Dec 26, 2023

@r-schmidt I have tested your code & there does seem to be an optimised & faster access to the PE (less CPU time is used). Only PE port 2 I could not test now. It's worth transferring the PR against the DEV branch, thanks for your effort!

@tueddy tueddy reopened this Dec 26, 2023
@tueddy
Copy link
Collaborator

tueddy commented Dec 26, 2023

Follow up #283

@tueddy tueddy closed this Dec 26, 2023
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.

2 participants