DigitalReader last_pressed and last_triggered

[awonak]

• Refactor handler debounce code to use recommended ticks_diff()
• Rename DigitalReader.last_pressed to better DigitalReader.last_rising_ms
• Added wrappers last_pressed and last_trigger to the Button and DigitalInput classes.
• Updated README docs

This addresses the first part of #60 with the last_pressed() etc methods. A future PR will address the falling edge handler.

[awonak]

I realized that last_pressed() and last_triggered() would be more useful returning the "duration" since the last press/trigger instead of the timestamp. This helps avoid duplicating code to calculate duration in user scripts, which is how it would be used.

Docs updated here: https://awonak.github.io/EuroPi/generated/europi.html#europi.Button.last_pressed

[awonak]

Alright, this PR is good to go.

[roryjamesallen]

I'm not sure if this is a bug or an intended function, but if you haven't assigned a handler then the digital input (din or buttons) will return the current time in ticks even if pressed, as the resetting of its self.last_rising_ms is done in the bounce wrapper function.
Thinking about it this shouldn't be an issue as using .value() rather than handler functionality at the same time as last_pressed() is very unintuitive and doesn't have many, if any, use cases

[awonak]

That is a very good point. I would argue that if a script calls last_pressed() and does not have handler defined, then that is a user programming error. However, last_pressed() could certainly be called before the handler function has been called, with last_rising_ms still containing the default value of 0.

Currently, when the default value is 0, last_pressed() gives the present ticks_ms(), which itself is an arbitrary value and does not accurately represent the duration since last pressed.

If the default value of last_rising_ms is set to ticks_ms() at init time, then script code will interpret that as detecting a rising edge event when the script starts. For example, the Polyrhythm Sequencer show_menu_header() method would run when the script first starts. Not a bad thing in this example, but you could imagine some behavior running at startup that is undesirable.

Another potential solution is to set the default last_rising_ms = None instead of 0 and refactor the last_pressed() method to return None if the rising edge has not yet been detected. This requires a little more code to use the method. Here's an example how a snippet from polyrhythm_seq.py script's show_menu_header() would be refactored:

def show_menu_header(self):
    # Returns None if unpressed, otherwise returns the duration since last press.
    last_pressed = b1.last_pressed()
    if last_pressed and last_pressed < MENU_DURATION:
        oled.fill_rect(0, 0, OLED_WIDTH, CHAR_HEIGHT, 1)
        oled.text(f"{self.pages[self.page]}", 0, 0, 0)

I'll think about this some more and graciously welcome any other suggestions.

[awonak]

Another proposal, last_pressed() returns duration or throws an error if it has not yet been pressed.

# europi.py
class HandlerNotYetCalled(Exception):
    pass

...

def _duration_since_last_rising(self):
    """Return the duration in milliseconds since the last trigger."""
    if self.last_rising_ms is None:
        raise HandlerNotYetCalled
    return time.ticks_diff(time.ticks_ms(), self.last_rising_ms)


# polyrhythm_seequencer.py

...

def show_menu_header(self):
    try:
        if b1.last_pressed() < MENU_DURATION:
            oled.fill_rect(0, 0, OLED_WIDTH, CHAR_HEIGHT, 1)
            oled.text(f"{self.pages[self.page]}", 0, 0, 0)
    except HandlerNotYetCalled:
        pass

[roryjamesallen]

The second solution looks much better to me! I think it's best to consider this a programming error and make the user aware of it rather than make it more difficult to use the method properly (your first example in the polyrhythm script)

[awonak]

I have updated the code and docs to match the second proposal which throws an exception prior to handler being first called.

https://github.com/Allen-Synthesis/EuroPi/blob/a64d9a04b2acb3914943b21eee1852ef98d2c479/software/README.md#button

If this seems like too much wrapper code, then we can revert back to last_pressed() simply returning ticks_ms of last pressed or None prior to being pressed and let the script author decide how to handle this themselves.

[awonak]

Reverted back to simply returning ticks_ms of the last button press, or 0 prior to being pressed.

https://github.com/Allen-Synthesis/EuroPi/blob/3e5012a675af13fef86545fe9d6441f025dfd27f/software/README.md#button

I think this is much cleaner and less error prone.

[roryjamesallen]

Is this one (and the IRQ refactor) ready to go now? All seems to be working great when I run it!