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 interrupt support #30

Merged
merged 4 commits into from
Jan 14, 2025
Merged

Added interrupt support #30

merged 4 commits into from
Jan 14, 2025

Conversation

yacobucci
Copy link
Contributor

@yacobucci yacobucci commented Jan 7, 2025

Remove NPIEN and AIEN from enable command.
Made separate control functions for enabling interrupts. Exposed new variables for setting enabled functions.

Added persist, no persist threshold high and low, and ALS threshold high and low properties for setting boundaries.

Interrupts are off by default so users have a chance to set thresholds and persist values before they receive an interrupt. Otherwise, with thesholds at 0 interrupts always fire until set or cleared. When both are enabled users can see unexpected interrupts if only one style is expected but both are on.

Tested on Raspberry Pi 5.

Closes #29

Remove NPIEN and AIEN from enable command.
Made separate control functions for enabling interrupts.
Exposed new variables for setting enabled functions.

Added persist, no persist threshold high and low, and ALS threshold high and low
properties for setting boundaries.

Interrupts are off by default so users have a chance to set thresholds
and persist values before they receive an interrupt. Otherwise, with thesholds at
0 interrupts always fire until set or cleared. When both are enabled users can
see unexpected interrupts if only only style is expected but both are on.

Tested on Raspberry Pi 5.
@yacobucci yacobucci mentioned this pull request Jan 7, 2025
Copy link
Member

@anecdata anecdata left a comment

Choose a reason for hiding this comment

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

All of the low and high threshold comments say "exceed", I don't think that's right. Outside the limits I believe is how it works (lower than the low, or higher than the high), though the datasheet also uses "exceed" to describe it.

edit: confirmed through testing, just for clarity of comments perhaps not use "exceed" to describe the low boundaires

@anecdata
Copy link
Member

anecdata commented Jan 8, 2025

Do you have a simple example from init to setting up the thresholds and reading values? Is there any special sequence of disabling/enabling the board or interrupts, or can the thresholds be changed at any time?

edit: from the datasheet and testing, it appears that interrupt characteristics can be set whether the ALS is enabled or not.

Looks good so far, I'm seeing interrupt pin triggered for raw 16-bit values that are outside whichever (or both) of the persist/no-persist range(s) that are enabled.

@yacobucci
Copy link
Contributor Author

All of the low and high threshold comments say "exceed", I don't think that's right. Outside the limits I believe is how it works (lower than the low, or higher than the high), though the datasheet also uses "exceed" to describe it.

edit: confirmed through testing, just for clarity of comments perhaps not use "exceed" to describe the low boundaires

I'll update that language, I likely just pulled it from the datasheet, but could be clearer.

@yacobucci
Copy link
Contributor Author

Do you have a simple example from init to setting up the thresholds and reading values? Is there any special sequence of disabling/enabling the board or interrupts, or can the thresholds be changed at any time?

edit: from the datasheet and testing, it appears that interrupt characteristics can be set whether the ALS is enabled or not.

Looks good so far, I'm seeing interrupt pin triggered for raw 16-bit values that are outside whichever (or both) of the persist/no-persist range(s) that are enabled. The thresholds and function enablement can be changed at any time. Status can be read at any time too. The device will adjust accordingly and will continue to operate until reset.

Here is a simple program to demonstrate usage (it's not commented well but shows how they operate):

import board
import adafruit_tsl2591
import RPi.GPIO as GPIO
import time
import logging
import signal
import sys
import argparse

logger = logging.getLogger(__name__)

class Interrupt:
    def __init__(self, sensor, pin, logger):
        self._sensor = sensor
        self._pin = pin
        self._log = logger

    def interrupt(self, channel):
        self._log.debug("called back in interrupt")
        if GPIO.input(self._pin):
            self._log.debug("rising edge")
        else:
            self._log.debug("falling edge")
        status = self._sensor._read_u8(0x13)
        if status & 0x20:
            self._log.debug("NPINTR is ON")
        if status & 0x10:
            self._log.debug("AINT is ON")
        self._log.debug('status is %s', bin(status))
        raw = self._sensor.raw_luminosity
        self._log.debug("Luminosity (%s, %s)", raw[0], raw[1])

def shutdown(sensor):
    GPIO.cleanup()
    sensor.disable_interrupt(adafruit_tsl2591.ENABLE_NPIEN)
    sensor.nopersist_threshold_low = 0
    sensor.nopersist_threshold_high = 0

    logger.debug('shutting down...')
    status = sensor._read_u8(0x00)
    logger.debug('state - NPINTEN:%s AINTEN:%s', status & 0x80, status & 0x10)
    logger.debug('thresholds - low:%s high:%s', sensor.nopersist_threshold_low,
                 sensor.nopersist_threshold_high)

def make_shutdown(sensor):
    def _signal_handler(sig, frame):
        shutdown(sensor)
        sys.exit(1)

    return _signal_handler

def main():
    parser = argparse.ArgumentParser(description='tsl2591.py: Interrupt Test')
    parser.add_argument(
        '--low',
        help='Low threshold',
        type=int,
        default=0,
    )
    parser.add_argument(
        '--high',
        help='High threshold',
        type=int,
        default=37889,
    )
    parser.add_argument(
        '--period',
        help='Wait period',
        type=int,
        default=10,
    )
    parser.add_argument(
        '--pin',
        help='Input pin',
        type=int,
        default=26,
    )
    args = parser.parse_args()
    logging.basicConfig(filename='tsl2591.log', level=logging.DEBUG,
                        format='%(levelname)s %(asctime)s : %(message)s')
    sensor = adafruit_tsl2591.TSL2591(board.I2C())
    signal.signal(signal.SIGINT, make_shutdown(sensor))
    signal.signal(signal.SIGTERM, make_shutdown(sensor))

    pin = args.pin
    GPIO.setmode(GPIO.BCM)
    GPIO.setup(pin, GPIO.IN, pull_up_down=GPIO.PUD_UP)
    intr = Interrupt(sensor, pin, logger)
    GPIO.add_event_detect(pin, GPIO.FALLING, callback=intr.interrupt)

    logger.debug("On Start luminosity: %s", sensor.raw_luminosity)
    
    low = args.low
    high = args.high
    logger.debug('Adding no persist thresholds: (%s, %s)', low, high)
    sensor.nopersist_threshold_low = low
    sensor.nopersist_threshold_high = high

    # turn on nopersist
    logger.debug('Turning on interrupts...')
    sensor.enable_interrupt(adafruit_tsl2591.ENABLE_NPIEN)

    try:
        while True:
            raw = sensor.raw_luminosity
            status = sensor._read_u8(0x00)
            logger.debug('Luminosity (%s, %s) - NPINTEN:%s AINTEN:%s', raw[0], raw[1],
                         status & 0x80, status & 0x10)
            time.sleep(args.period)
            logger.debug('waking up - clear interrupts')
            if low <= raw[0] <= high:
                logger.debug('DO NOT EXPECT an interrupt...')
            else:
                logger.debug('EXPECT an interrupt...')
            sensor.clear_interrupt(adafruit_tsl2591.CLEAR_ALL_INTERRUPTS)
    except KeyboardInterrupt:
        shutdown(sensor)

if __name__ == '__main__':
    main()

@yacobucci
Copy link
Contributor Author

yacobucci commented Jan 8, 2025

Do you have a simple example from init to setting up the thresholds and reading values? Is there any special sequence of disabling/enabling the board or interrupts, or can the thresholds be changed at any time?
edit: from the datasheet and testing, it appears that interrupt characteristics can be set whether the ALS is enabled or not.
Looks good so far, I'm seeing interrupt pin triggered for raw 16-bit values that are outside whichever (or both) of the persist/no-persist range(s) that are enabled. The thresholds and function enablement can be changed at any time. Status can be read at any time too. The device will adjust accordingly and will continue to operate until reset.

Is there any special sequence of disabling/enabling the board or interrupts, or can the thresholds be changed at any time?

Thresholds can be updated at any time. And NPIEN and AIEN enabled or disabled at any time. The device will continue to operate with the latest parameters until powered off or SRESET.

Where it might be confusing is when enabling interrupts and THEN setting thresholds. If enabled with (0, 0) then the device will interrupt. Setting thresholds first is easier to reason about.

@anecdata anecdata requested a review from a team January 8, 2025 15:43
@anecdata
Copy link
Member

anecdata commented Jan 8, 2025

@yacobucci Those new descriptions are much clearer (clearer than the datasheet too), thank you. Sorry, thought of one other thing... the persist value is weird, had to look at p.18 of the datasheet to see that the value isn't the number of ALS cycles (the text of the datasheet is misleading without the table). It increases by 1 for a few, then by 5 up to 60. Maybe something like "higher values increase the number of consecutive out-of-range ALS cycles necessary to generate an interrupt". It's probably not worth a ton of new constants.

I think anyone using the thresholds will be consulting the datasheet anyway to look at the curves, or establish empirically what they want the thresholds and persist to be (duration time is dependent on integration time too). I doubt many people think in raw luminosity.

@yacobucci
Copy link
Contributor Author

@yacobucci Those new descriptions are much clearer (clearer than the datasheet too), thank you. Sorry, thought of one other thing... the persist value is weird, had to look at p.18 of the datasheet to see that the value isn't the number of ALS cycles (the text of the datasheet is misleading without the table). It increases by 1 for a few, then by 5 up to 60. Maybe something like "higher values increase the number of consecutive out-of-range ALS cycles necessary to generate an interrupt". It's probably not worth a ton of new constants.

I think anyone using the thresholds will be consulting the datasheet anyway to look at the curves, or establish empirically what they want the thresholds and persist to be (duration time is dependent on integration time too). I doubt many people think in raw luminosity.

Good suggestion. I think I have a better way to explain persist. I'll get something up shortly.

Improve persist documentation. Full description of accepted values.

Code consistency: other contrained setters assert, persist now asserts
Input arguments are called `val` throughout.
Copy link
Member

@anecdata anecdata left a comment

Choose a reason for hiding this comment

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

Changes look good, no ambiguity, running the updates now and things are working as expected. I'm going to approve this, but leave open for now in case someone else wants to look at the code or do more testing. I'll be afk for a couple weeks soon, but if it's still open when I get back, I can merge and release. Thanks for this very useful addition @yacobucci !

Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

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

This looks good to me as well. Thanks for working on this @yacobucci, and thanks @anecdata for review & testing

@FoamyGuy FoamyGuy merged commit 9125f77 into adafruit:main Jan 14, 2025
1 check passed
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Jan 15, 2025
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.

Support interrupts
3 participants