dmadison / ServoInput

Interrupt-driven servo decoder library for Arduino
GNU Lesser General Public License v3.0
22 stars 10 forks source link

Needs lost signal timeout function #22

Closed EclipseBoom closed 7 months ago

EclipseBoom commented 2 years ago

Additional function request:

For reliability in real applications it is important to know if the input signal has been lost due to a broken wire or interference or whatever. Basically a timeout function from the last valid pulse reception on each channel.

Currently the user must keep track of the time since the last pulse reception for each channel since getPulse() keeps returning the last valid value. Since the library is already interfacing with the microsecond timer it would seem easy to add a timestamp on reception of a valid pulse for each input. Then something analogous to available() could be used to check if the pulses are still coming in OK. Perhaps a status() or timeout() or something like that method could be added to allow the user a quick way to make sure all is OK and take safety action if not.

Thanks, Chris

EclipseBoom commented 2 years ago

Perhaps just return the uS since the last valid pulse. That would serve the timeout function as well as an easy way to calculate pulse frequency.

A quality measurement might be useful is some noisy situations. Something like a pulse error rate. Not as critical I think but might be useful in some situations.

dmadison commented 2 years ago

Hmm. I can see the utility, but I think this is going to be tricky to implement without adding some bulk to the ISR.

Currently the pulse is only validated when it's retrieved, which works since the ISR is constantly updating the current pulse duration as it's triggered. If we want to know when the last valid pulse was received, we need to either validate the pulse within the ISR (which is not very performative) or save the timestamp when the pulse is first retrieved (which is not very accurate to what's actually going on).

A quality measurement might be useful is some noisy situations. Something like a pulse error rate.

That seems tricky, because you would need to start making assumptions about the control signal. Even something simple, like counting 'valid' vs. 'invalid' pulse durations, has the same issue as above since you would need to do the tracking within the ISR otherwise if the user isn't polling frequently it won't be accurate.

EclipseBoom commented 2 years ago

True on the ISR timing especially since some RC receivers don't have much or any space between the edges of the pulses on the different channels. However you are already fetching the timestamp of the falling edge of the pulse in order to calculate the width and validate it. All that is needed extra is to store that final timestamp if the pulse is valid.

The alternative is what I'm currently doing which is to save an application-level timestamp when a new pulse comes in and available() is checked by the application. This is even less accurate...

In my case the timeout criteria is ~25 pulses missed. So ~500mS with no pulses constitutes "signal lost" and I shut everything off. So uS precision is not needed in my case of detecting no pulses "for too long".

dmadison commented 2 years ago

All that is needed extra is to store that final timestamp if the pulse is valid.

Right, but the 'if the pulse is valid' bit means that we need to run the validation function from within the ISR, which isn't ideal. I don't think it would break anything I'm just not too fond of the extra overhead.

Any recommendations on a name for the timestamp 'get' function?

EclipseBoom commented 2 years ago

Not really necessary to validate. Just store a candidate like you currently do with the pulse width. Then update the timestamp at the same time you update the pulse value. Just a suggestion.

dmadison commented 2 years ago

It's trickier than it seems, since it strongly couples the two values. You'd have to add a couple of variables to keep track of whether it's been evaluated or not or sprinkle tests throughout the various functions which reference that data. It's also not much of an upgrade over the current situation because it requires constant polling to check whether the value is valid before it's overwritten. As long as it's not an issue I'm just going to make it simple and stick it into the ISR.

I've added this functionality to the timeout branch. Let me know if it works for you.

dmadison commented 7 months ago

It's been over a year without an update, I assume you no longer need this feature. Feel free to reopen if you'd like to approach this again.