Skip to content

Analog threshold improvements #141

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

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from

Conversation

alustig3
Copy link
Contributor

@alustig3 alustig3 commented Mar 23, 2025

Added an option to provide upper and lower thresholds for triggering the rising and falling events. This is particularly useful if you have a noisy signal and want to implement a software Schmitt trigger.

Also added a helper function change_threshold() for more easily adjusting the threshold after a task has started.

@alustig3 alustig3 changed the base branch from master to dev March 23, 2025 23:35
@ThomasAkam
Copy link
Collaborator

Thanks Andy. I think supporting seperate thresholds for rising and falling edge generation is desirable but I think there may be annother way of implementing it which is more general and possibly simpler. Rather than modifying the Analog_threshold class to support seperate thresholds for rising and falling edge generation, we could modify the Analog_input class to allow multiple thresholds to be specified. Syntax could be:

rising_threshold = hw.Analog_threshold(threshold=2000, rising_event='rising_edge')
falling_threshold = hw.Analog_threshold(threshold=1000, falling_event='falling_edge')
analog_input  = hw.Analog_input(
    pin='X5', name='Analog 1', sampling_rate=1000,  thresholds=[rising_threshold, falling_threshold]
)

or alternatively:

analog_input  = hw.Analog_input(
    pin='X5', name='Analog 1', sampling_rate=1000,  thresholds=[
        {'threshold':2000, 'rising_event': 'rising_edge'},  
        {'threshold':1000, 'falling_event': 'falling_edge'}
    ]
)

Supporting multiple thresholds on a single analog input was part of the rationale for seperating out the threshold into a seperate class. An advantage of this approach is that it would support implementing as many different threshold values as required, each capable of generating rising and/or falling edges, giving the user very rich control of event generation. A dissadvantage is it would incur somewhat higher overheads than your current implementation for a basic Schmitt trigger. What do you think?

@alustig3
Copy link
Contributor Author

Yes, I like your suggestion and the first syntax. I took a swing at implementing it.

An analog input can now have triggers added to it. A trigger can have a rising and/or falling event. I made a ValueTrigger that takes a single value and a SchmittTrigger that takes an upper and lower bound

here is an example task:

from pyControl.utility import *
from devices import Rotary_encoder
from pyControl.hardware import ValueTrigger, SchmittTrigger


high_trigger = ValueTrigger(threshold=9000, rising_event="crossed_high")
mid_trigger = ValueTrigger(threshold=8000, rising_event="crossed_mid", falling_event="crossed_mid")
low_trigger = ValueTrigger(threshold=7000, falling_event="crossed_low")
schmitt_trigger = SchmittTrigger(bounds=(1000, 5000), rising_event="run_start", falling_event="run_stop")

running_wheel = Rotary_encoder(
    name="running_wheel",
    sampling_rate=40,
    output="velocity",
    triggers=[high_trigger, low_trigger, mid_trigger, schmitt_trigger],
)
states = [
    "idle",
]
events = [
    "run_stop",
    "run_start",
    "crossed_low",
    "crossed_mid",
    "crossed_high",
    "change_during_session",  # trigger this event from the controls dialog
]

initial_state = "idle"


def idle(event):
    if event == "change_during_session":
        high_trigger.set_threshold(-1000)
        schmitt_trigger.set_bounds((50, 200))

Previously, from what I could tell, threshold values could not be reconstructed from the data? I think you'd have to look at the task file or hardware definition. Now, especially since it is easier to adjust thresholds during a task, I think there should automatic logging of the thresholds. I did this crudely by adding a “trigger” subtype to the print MsgType, but hopefully you'll have a more elegant solution. Ideally the log should track the thresholds just like variables are tracked, i.e. it should show run_start values, all changes in the session, and run_end values.

@ThomasAkam
Copy link
Collaborator

Thanks Andy, this looks good. A couple of thoughts:

  • To avoid breaking backwards compatibility with task files we should still support the Analog_input arguments threshold, rising_event and falling_event to instantiate a single threshold.
  • To avoid increasing the size of the framework code too much we could move the SchmidtTrigger class to a seperate device driver file.
  • I might be inclined to keep the name Analog_threshold for the class you are currently calling ValueTrigger, as that would keep the terminology consistent between the argument name of the Analog_input class and the class name used for specifying multiple thresholds.
  • I agree we should have threshold values specified in the data file if they can be changed during the run. Not sure what the best type & subtype is for this. will think about it.

Also, I have updated the dev branch to include the latest commits on master, could you update the pull request so it is based ont the current dev branch to make it easier to see which files have changed.

- can now provide a upper and lower threshold
- raise an error if a rising or falling event is provided without a threshold
- raise errors if threshold(s) not provided correctly
- add ability to change threshold(s) after task has started
@alustig3 alustig3 force-pushed the analog-threshold-improvements branch from fbe1609 to 1d57995 Compare March 27, 2025 16:58
@alustig3
Copy link
Contributor Author

alustig3 commented Apr 4, 2025

  • thanks for updating dev, I've rebased this branch, so it should be easier to see changes
  • Analog_input() is now backwards compatible (original example/running_wheel.py should still work)
  • SchmittTrigger is now a separate device file
  • ValueTrigger name has been reverted back to Analog_threshold
  • I created a new type threshold with a subtype set
    • since this solution only uses one subtype (set), maybe there shouldn't subtype at all (like with STATE messages), or maybe subtypes value and schmitt would be useful?
    • Maybe trigger is a better type name than threshold?
    • I'm not sure what other potential subtypes might be needed. I thought about having a run_start and run_end but it would maybe require (or at least be easier to implement with) a separate thresholds class like the variables currently have with v., but creating a new class just for triggers seemed excessive. With the current solution, since the triggers are set upon initialization, the data gets logged at the start, achieving the same outcome/conveying the same information that would be given by a run_start subtype
  • I updated example/running_wheel.py to use the new syntax and demonstrate having multiple triggers

The following task code can be used for you to testing:

from pyControl.utility import *
from devices import Rotary_encoder, SchmittTrigger
from pyControl.hardware import Analog_threshold


high_trigger = Analog_threshold(threshold=9000, rising_event="crossed_high")
mid_trigger = Analog_threshold(threshold=8000, rising_event="crossed_mid", falling_event="crossed_mid")
low_trigger = Analog_threshold(threshold=7000, falling_event="crossed_low")
schmitt_trigger = SchmittTrigger(bounds=(1000, 5000), rising_event="run_start")

running_wheel = Rotary_encoder(
    name="running_wheel",
    sampling_rate=40,
    output="velocity",
    triggers=[high_trigger, low_trigger, mid_trigger, schmitt_trigger],
)
states = ["idle"]
events = [
    "run_stop",
    "run_start",
    "crossed_low",
    "crossed_mid",
    "crossed_high",
    "change_during_session",  # trigger this event from the controls dialog
]

initial_state = "idle"


def idle(event):
    if event == "change_during_session":
        high_trigger.set_threshold(-1000)
        schmitt_trigger.set_bounds((50, 200))

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