more user friendly?

[aster94]

hello,
this lib is great, thak you to have continued the good work of the first author

but i was wandering if it would be possible to make it more user friendly, for example, instead of:

ClickEncoder::Button buttonState = encoder->getButton();

i would like to use (as all the library that are around):

ClickEncoder myRE;
buttonState = myRE.getButton();

i know it would need some work of class changes but then the library would be perfect

cheers

[soligen2010]

I'm not sure what you are proposing as your second example is incomplete because the mrRE object needs input parameters to be initialized and buttonState is not declared.

what you need to do is something like this

ClickEncoder myRE = ClickEncoder(BUTTON_INPUT_PIN,false);
ClickEncoder::Button buttonState;

{other code may be here}

buttonState = myRE.getButton();

{more other code may be here}

This style is already possible its just your preference on how to do things in C++. I don't know of any other way to do things, so if you can post your proposed changes to the library to achieve what you are after, maybe then I will understand what you are after.

[septillion-git]

The problem lies in the fact there is no begin() method (or other initialization method), which is normal in Arduino, but all init is done in the constructor. Because you can't call pin function (reliable) in a constructor of a global you need to construct the object in a function. But to make it global available a object pointer is created global which is pointed to a valid constructed object in setup(). But because you need to dereference the pointer in order to call the methods of the object you end up with the foo->method() (or (*foo).method() instead of foo.method() .

So in that sense it would be neater (and more in line with Arduino practices) if the object had a begin() method to do the initialization.

Second part has to do with the encapsulation of the Button enum in the ClickEncoder class. That is something I like because it keeps stuff nice and organized without the possibility thing interfere because Button, Pressed, Held etc are all very general keywords. You can make a shorter typedef in your code (not the library) if you're sure things don't interfere.

typedef ClickEncoder::Button Button;

Now you can just do

Button b = encoder->getButton();
//....
if(b == Button::Pressed){
  //stuff
}

[aster94]

@septillion-git got the problem
For example a nice logic to deference (if it is really needed) the pointer is here: https://github.com/bolderflight/MPU9250

To be in line with the arduino style i imagine the constructor should appear this way:
ClickEncoder RE (uint8_t pinA, uint8_t pinB, uint8_t pinSwitch, uint8_t steps)

Than we could use the object this way:
RE.gerButton() ecc

It would need a little bit of changes, that is sure
Maybe if i have time during christmass holiday i would try to make a fork if you aren t interested to do this

[soligen2010]

Aster, Can you post more complete examples of what you area after, or perhaps say what actually needs to be changed in thelibrary. What you are posting seems incomplete, and/or not valid C++.

The constructor you suggest is basically what is already there, except you left off the bool active, which is needed to address positive or negative logic depending on how things are wired.

I don't know what you are getting at with

RE.gerButton() ecc

this doesnt look like valid C++ to me, or I am missing your intent.

Like I said earlier, you can already use the dot notation with this library instead of dereferencing pointers. It's just that the author of the examples choose to use the dereferencing style for some reason. None of my personal sketched use the dereferencing style.

I seems like the only gap the is to have an empty or abbreviated constructor and a begin method to configure things. Frankly I don't see the point in doing this, but I have no objection to adding this if you want to work on it as long as everything stays backwards compatible, you add appropriate example sketches, and make sure that DigitalButton and AnalogButton work the same way.

Don't let the next paragraph discourage you of this is what you want to do, but if I got it all wrong, take the opportunity to educate me.

It doesn't seem to me that this change adds anything useful to the functionality, and I don't see a natural division of what should be in the constructor vs begin, so I don't really see the point in doing this just to have a begin method. In my mind less is better (simpler) unless there is functionality to be gained. Maybe you can explain why this is the way things should be done. or what useful functionality it would add. It is true many libraries have a begin, but I cant find where it is mentioned that having a begin is a standard. In fact, on the arduino site article on how to write a library, the example they use doesn't have a begin. The only reason I can see for a begin is to be able to dynamically share the pin resources, to turn on/off a device, or dynamically change something. I don't really see these as being applicable to encoders.

Just as a side note if you intend to work on this library. DigitalButton and AnalogButton are not really done they way they should be. I just never had reason to go back and refactor things. Ideally both of these would be their own independent class and ClickEncoder would dynamically allocate one of these as a helper class if button logic was called for, or they could be used stand alone. This approach would save memory for sketches that had encoder without buttons, or buttons without encoder without having to use a pre-compiler directive to compile without buttons. It's a cleaner way to do things, but it is a really big fundamental change in how the library is structured. And as it works as is, I never had reason to make such a large change.

[aster94]

@soligen2010 there is no real advantage in doing what i am proposing. I just pointed out which is the arduino style
if this change will be made every kind of compatibility would be lost, for this reason if i m really going to do this i will make my fork, but really for now i don't have time, i will put this in my todo list

[soligen2010]

Pull request merged

[aster94]

Today in the forum i found a document on how to write libraries with what i called the "arduino style" you may find it usuful for future development
https://www.arduino.cc/en/Reference/APIStyleGuide