Go Down

Topic: New EdgeDebounce feature not working (Read 2580 times) previous topic - next topic

jbellavance

#15
Aug 24, 2017, 06:53 pm Last Edit: Aug 24, 2017, 07:02 pm by jbellavance
Quote
dodgy defines
Code: [Select]
dodgy
adjective BRITISH informal
dishonest or unreliable.
"a dodgy secondhand car salesman"
potentially dangerous.
"activities like these could be dodgy for your heart"
of low quality.
Really?
Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

BulldogLowell

Code: [Select]
dodgy
adjective BRITISH informal
dishonest or unreliable.
"a dodgy secondhand car salesman"
potentially dangerous.
"activities like these could be dodgy for your heart"
of low quality.
Realy?
I'm not British...

here we use the term to describe anything questionable, dubious, arguable or just plain F'ed up in quality or character!

The slang was not to impugn your works;  It's a long time argument using #define directives for constants like you are in your example code

;)

Jiggy-Ninja

Quote
Code: [Select]
for (byte i = 0; i < w; i++) debounceDontCare = debounceDontCare << 1 | 0;
What point is there to ORing a number with 0? That's like adding 0 or multiplying by 1.

I get what you're intending to do, it's just a senseless way of doing it.

The loop is also unnecessary. Try this on for size. :)
Code: [Select]
debounceDontCare = ~((1<<w)-1);

Quote
Since the .rose() and .fell() methods only returns the values of MYrose and MYfell respectively, I sincerely can't figure out where the information is lost.
No, they most certainly do NOT only return the values of some variables. Take a closer look at your own functions:

Code: [Select]
//statusMethods=====================================================
bool EdgeDebounce::closed() { update(); return  bool(MYstatus); }
bool EdgeDebounce::open()   { update(); return bool(!MYstatus); }

//transition methods========================================
bool EdgeDebounce::rose() { update(); return MYrose; }
bool EdgeDebounce::fell() { update(); return MYfell; }

You have a call to update() is each of those functions, and update() has a call to debounce(), the routine that reads the switch state. That's what's causing your problem.

An edge is not like a level. Levels can be held for long periods of time. Edges are transient, existing only for a single instant of time and are gone the next time you debounce the switch. Suppose you call rose() on the instant a falling edge actually happens. Your code will pick up the falling edge, check for a rising edge, and return false. Now microseconds later you call the fell() function and check the switch again for a falling edge. Except the falling edge already happened. You missed it, and now it will read a stable level and check it for the falling edge. Obviously it will also return false.

Since it will be random which call the edges will occur on, I predict that with that sketch you will only detect the edge properly about 50% of the time. A cursory analysis of the output string you posted confirms that (Only 11 out of 23 correct sequences).

Call the update() function once per run of loop. Your convenience functions (open, closed, rose, fell) must only read the internal state variables, and should not call any functions that modify them. A good way to enforce this is to declare the functions const in your header.

Code: [Select]
   bool closed() const;                      //Returns true if the switch is closed
    bool open() const;                        //Returns true if the switch is open
    bool rose() const;                        //Returns true if on the rising edge of the signal (Just closed)
    bool fell() const;  


By declaring the function const, you are telling the compiler that that function will not modify any of the classes member variables. It cannot assign or modify any member variables, and the only member function you can use are ones that are likewise declared const. The compiler will throw an error if you violate that rule and use modifying code in a const function.

C++ is by design a strict language. There are hard rules defining things like type conversions, and you can place restrictions on your own code by using keywords like const. If you're not used to that, its strictness can feel suffocating and limiting. Once you're familiar with it, you can use that strictness to save you from your own mistakes, just like a rail can prevent a car from plummeting off the edge of a cliff.

When creating a class, it is good practice to use the const keyword for any function that you intend to not change the state of the class. "Getter" functions that only return local variables (like getSensitivity) are perfect to do that to.

BulldogLowell

An edge is not like a level. Levels can be held for long periods of time. Edges are transient, existing only for a single instant of time and are gone the next time you debounce the switch. Suppose you call rose() on the instant a falling edge actually happens. Your code will pick up the falling edge, check for a rising edge, and return false. Now microseconds later you call the fell() function and check the switch again for a falling edge. Except the falling edge already happened. You missed it, and now it will read a stable level and check it for the falling edge. Obviously it will also return false.
sounds right:

Your problem may be that the opposing edge event may have happened in the opposite call (i.e. rose() vs. fell())


Jiggy-Ninja

#19
Aug 24, 2017, 07:20 pm Last Edit: Aug 24, 2017, 07:21 pm by Jiggy-Ninja
In my own suite of library code, I have separate classes for debouncing and edge detection. The debounce_filter class filters the raw button input and outputs the level with bounce transitions removed. The debounced output is then fed into the separate edge_detector class, which returns true when there's an edge (new value different from the previous value) and false when there isn't.

The direction of the edge can be discerned by the level of the debounced signal. It it's HIGH when an edge happens, it was a rising edge. It it's LOW, falling.

Separating them like this lets me more easily envision how they interact, and also makes the reuse more modular. A digital sensor (like a Hall effect switch) might need the edge detection code, but won't need debouncing. I can easily use the edge_detector class without needing to strip anything out.

jbellavance

Hi, Thanks for seeing this. your explanation is clear.

I will rewrite the thing so that the call from the Sketch will look like:
Code: [Select]
if (button.close() && button.rose()) {
  /Your code here
}

I also like the idea of disabelling the debouncer altogether.

I will get back soon.

Jacques

Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

jbellavance

#21
Aug 24, 2017, 07:54 pm Last Edit: Aug 24, 2017, 07:58 pm by jbellavance
Hi,

Changed .cpp to:
Code: [Select]
byte EdgeDebounce::update() {
  byte newStatus = debounce();
  if (MYmode == PULLUP) newStatus = !newStatus;
  if (MYstatus == OPEN && newStatus == CLOSED) { MYrose = true; Serial.print('.'); }
  else                                           MYrose = false;
  if (MYstatus == CLOSED && newStatus == OPEN) { MYfell = true; Serial.print(':'); }
  else                                           MYfell = false;
  MYstatus = newStatus;
  return newStatus;
}//update-------------------------------------------------------------------------

//statusMethods=====================================================
bool EdgeDebounce::closed() const { return MYstatus; }
bool EdgeDebounce::open()   const { return !MYstatus; }

//transition methods========================================
bool EdgeDebounce::rose() const { return MYrose; }
bool EdgeDebounce::fell() const { return MYfell; }


And sketch to:
Code: [Select]
#include <EdgeDebounce.h>

EdgeDebounce test(4, PULLUP);

void setup() {
  test.begin();
  Serial.begin(9600);
}

void loop() {
  int theState = test.update();
  if (test.rose()) Serial.print(',');
  if (test.fell()) Serial.print(';');
}


It all works now.

Thanks

Jacques
Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

jbellavance

Quote
debounceDontCare = ~((1<<w)-1);
It took me a little while just to understand what the original algorithm:
Code: [Select]
bool_t DebounceSwitch2()
{
Static uint16_t State = 0;
State=(State<<1) | !RawKeyPressed() | 0xe000;
If(State==0xf000)return TRUE;
Return FALSE;
}
actually did.

I will need some time to figure out your shorthand notation... Learning opportunity :smiley-draw:

Jacques
Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

Jiggy-Ninja

#23
Aug 24, 2017, 08:56 pm Last Edit: Aug 24, 2017, 08:56 pm by Jiggy-Ninja
It's not shorthand, it's an expression. Suppose you pass in 16 for the variable w.

1UL<<w left-shifts a 1 by 16 spaces, producing the number 0x00010000. Note the UL suffix I added here. It's important and I forgot to put it in the original post.

From this the quantity 1 is subtracted, producing 0x0000FFFF.

~ is a bitwise inversion, producing 0xFFFF0000.

Try it again with 12.

1UL<<12 = 0x00001000
0x00001000 - 1 = 0x00000FFF
~0x00000FFF = 0xFFFFF000

jbellavance

Yes,

I just did some tests in Word (easy to jot down 0s and 1s)
Code: [Select]
00000000000000010000000000000000
00000000000000001111111111111111
11111111111111110000000000000000


Thanks again
Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

Go Up