Add callback function

[Mike4QL]

This will allow the caller to indicate when a double reset will be activated.

[Mike4QL]

I like simplicity, especially if it’s accompanied by clarity. How about having the loop method return the waitingForDoubleReset value. You would then know the state of DRD in both setup (detectDoubleReset returns false) and loop with minimal changes and backward compatability.

Mike

From: Stephen Denne [email protected] Sent: 14 March 2018 08:27 To: datacute/DoubleResetDetector [email protected] Cc: Mike Robertson [email protected]; Author [email protected] Subject: Re: [datacute/DoubleResetDetector] Add callback function (#7)

@datacute commented on this pull request.

---

In src/DoubleResetDetector.cpphttps://github.com/datacute/DoubleResetDetector/pull/7#discussion_r174381518:

bool DoubleResetDetector::detectDoubleReset() {

    doubleResetDetected = detectRecentlyResetFlag();

    if (doubleResetDetected) {

           clearRecentlyResetFlag();

    } else {

           setRecentlyResetFlag();

           waitingForDoubleReset = true;

•          if (callback != nullptr) {
  

•                  callback(waitingForDoubleReset);
  

•          }
  
    }
  

I see, and I think that being able to indicate that it's 'live' is a nice feature to add. However a simpler solution would be to expose the condition used in loop() as a new public method something like:

bool DoubleResetDetector::timeToStop() {

return waitingForDoubleReset && millis() > timeout;

}

Then DoubleResetDetector doesn't need to know about the callback method, as you can use the response from drd.detectDoubleReset() to know that you're not a double reset but you've started waiting, and instead of calling drd.loop() in your example, you can have:

if (drd.timeToStop()) {

drd.stop();

drd_callback(false);

}

I think this would also make it easier to understand even without your callback.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/datacute/DoubleResetDetector/pull/7#discussion_r174381518, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ARujTpPqCukdV6f2revy6EuaeN_s1kfLks5teNRMgaJpZM4SoKfJ.

[datacute]

I'd prefer a method that returns true only once. The loop method can be changed to return true when it stops, false otherwise, but keeping the name 'loop' makes it more difficult to understand the code that is utilizing the returned value. When you come back to it trying to get up to speed quickly, you'll read and think "if drd loop ... what?" Seeing the right method name in the example code should make it clear why you have to call the method in the loop. Additionally, every byte of code saved (if you don't need the new feature) can be precious. I guess my "timeToStop" suggestion still leaves the reader thinking "stop what?". Not having to also call stop might be beneficial, but then the method is both a query and an action, making it hard to name, and risking misuse. I want to avoid users thinking "I don't need to know the result of the query so I'll remove the call" so a method name for a combined method should refer to the action. Something like "processTimeout"? (I also thought of "detectTimeout" which I like even though it sounds like a query.)

[Mike4QL]

I agree with all you’ve said here but I’m wondering if we’ve looped back around to using a callback.

• It is only called once whenever the status changes
• It can be named whatever is appropriate to its use.
• You keep all the code associated with its use in one place separate from the rest of the program.

Mike

From: Stephen Denne [email protected] Sent: 14 March 2018 11:05 To: datacute/DoubleResetDetector [email protected] Cc: Mike Robertson [email protected]; Author [email protected] Subject: Re: [datacute/DoubleResetDetector] Add callback function (#7)

I'd prefer a method that returns true only once. The loop method can be changed to return true when it stops, false otherwise, but keeping the name 'loop' makes it more difficult to understand the code that is utilizing the returned value. When you come back to it trying to get up to speed quickly, you'll read and think "if drd loop ... what?" Seeing the right method name in the example code should make it clear why you have to call the method in the loop. Additionally, every byte of code saved (if you don't need the new feature) can be precious. I guess my "timeToStop" suggestion still leaves the reader thinking "stop what?". Not having to also call stop might be beneficial, but then the method is both a query and an action, making it hard to name, and risking misuse. I want to avoid users thinking "I don't need to know the result of the query so I'll remove the call" so a method name for a combined method should refer to the action. Something like "processTimeout"? (I also thought of "detectTimeout" which I like even though it sounds like a query.)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/datacute/DoubleResetDetector/pull/7#issuecomment-372983432, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ARujTmLIDyn7d6ntKTxn-EUvF1DiG8a_ks5tePlVgaJpZM4SoKfJ.