add basic button support

[bushmango]

I added basic support for handling three of the four buttons when the watch is in Watchface mode.

This can be an example for others, or I'm happy to modify to make it part of the core base, as it is nice to use the buttons to change settings on some watchfaces.

Enjoy! Thanks for the fun product.

[synthead]

Oh hey interesting! It looks like we were both thinking the same thing! I also just submitted #165 😄

I think your PR has an interesting implementation, because it lives alongside core functionality without overriding anything. Ultimately, both changes can be merged without conflicts, which would give users the option to override specific buttons, or the implementation as a whole.

How would you feel about also adding a fourth button function, and putting showMenu(menuIndex, false); in it by default? That way, the user could even conditionally override the default behavior of pulling up the menu, if they wanted.

Also, how would you feel about following the existing function names a little closer, and calling them something like Watchy::handleButton1Press?

[synthead]

Also, how about following the button numbers in the Getting Started doc?

[bushmango]

I like all of this feedback, I was wondering what to call the buttons. It does make sense to let the user override the menu even if normally they shouldn't 😛

…

On Thu, May 12, 2022, 6:27 AM Maxwell Pray ***@***.***> wrote: Also, how about following the button numbers in the Getting Started doc? [image: image] <https://user-images.githubusercontent.com/820984/168064421-d470830a-3877-47d5-8df8-725c58486c59.png> — Reply to this email directly, view it on GitHub <#164 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACAM657QNVL5VS4TGUXJZFDVJTTILANCNFSM5VWMS7WA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[synthead]

Adding the aforementioned suggestions as a review 👍

It does make sense to let the user override the menu even if normally they
shouldn't 😛

A relatable example for this could be SW1's "confirm" functionality. If a watch face has a menu of its own (i.e. themes or backgrounds), not being able to hit the same button to confirm the menu selection could be a UX smell. Additionally, if the user presses the "confirm" button thinking that it would confirm a menu (as the doc states), and a second menu gets brought up without "closing" the first one, they might get lost in what just happened.

[synthead]

How about:

Suggested change

	button1();
	handleButton2Press();

This lines up with the function naming pattern that already exists, handleButtonPress.

Also, this is SW2 on the PCB, and here is SW2 documented in the Getting Started guide: https://watchy.sqfmi.com/docs/getting-started/

[synthead]

Suggested change

	button2();
	handleButton3Press();

Same deal, with this one being SW3:

[synthead]

Suggested change

	button3();
	handleButton4Press();

And this one 🙂

[synthead]

This will help keep the style consistent:

Suggested change

	void Watchy::button1()
	{
	}
	void Watchy::button2()
	{
	}
	void Watchy::button3()
	{
	}
	void Watchy::handleButton2Press() {}
	void Watchy::handleButton3Press() {}
	void Watchy::handleButton4Press() {}

[synthead]

Suggested change

	virtual void button1(); // override these methods to handle different non-menu button presses
	virtual void button2();
	virtual void button3();
	virtual void handleButton2Press(); // override these methods to handle different non-menu button presses
	virtual void handleButton3Press();
	virtual void handleButton4Press();

[aneeshdurg]

This is my 2 cents - maybe instead of referrng to buttons by numbers in the functions names we could refer to buttons by their default functionality? e.g. handleButtonBackPress, handleButtonMenuPress, handleButtonUp/DownPress

Really like this approach though! ❤️

[JollyWizard]

This is my 2 cents - maybe instead of referrng to buttons by numbers in the functions names we could refer to buttons by their default functionality? e.g. handleButtonBackPress, handleButtonMenuPress, handleButtonUp/DownPress

Really like this approach though! heart

This would create confusion if the dev/user wanted to swap the handedness of the watch. It would be more appropriate to use a clear coordinate naming system, such as handleButtonUpperLeft, handleButtonUpperRight, handleButtonLowerLeft, handleButtonLowerRight.

This also creates a coherency issue with vertical flips, but is tied semantically to the physical orientation of the device, without making unnecessary assumptions about use case.

[edent]

Is it possible to use this to update the watch face? I've updated the .cpp and .h file with the handleButton2Press(); etc code. If I add something like this to my code:

 void handleButton2Press(){
          pinMode(VIB_MOTOR_PIN, OUTPUT);
          digitalWrite(VIB_MOTOR_PIN, true);
          delay(150); 
          digitalWrite(VIB_MOTOR_PIN, false);
}

Then the Watchy vibrates when I press the button. But how can I get the display to change when a button is pressed?

Code like this doesn't work:

void handleButton4Press(){ 
          display.fillScreen(GxEPD_WHITE);
          display.setTextColor(GxEPD_BLACK);
          display.setFont(&latin8pt7b);

          display.setCursor(0, 10);
          display.println("Button 4 Pressed!");
}

Is there something else I need to do to get the screen to update?

[amsam0]

@edent judging from this line of code:

https://github.com/sqfmi/Watchy/blob/master/src/Watchy.cpp#L610

display.display() (WatchyDisplay::refresh()?) needs to be called for the display to update