Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add simple calculator face #454

Merged

Conversation

mcguirepr89
Copy link
Contributor

@mcguirepr89 mcguirepr89 commented Aug 31, 2024

How to use:

Important note: LONG PRESS MODE to move to next watch face

Flow:

Enter first number -> Select operator -> Enter second number -> View Results

How to read the display:

  • "CA" is displayed at the top to tell you that you're in the CAlculator
  • The top-right digit (1, 2, or 3) lets you know whether you're entering the
    first number (1), entering the second number (2), or viewing the results (3).
  • To the right of the top-right digit will show the number's sign. If the
    number is negative, a "-" will be displayed, otherwise it is empty.
  • The 4 large digits to the left are whole numbers and the 2 smaller digits
    on the right are the tenths and hundredths decimal places.

Entering the first number:

  • Press ALARM to increment the selected (blinking) digit
  • Press LIGHT to move to the next placeholder
  • LONG PRESS the LIGHT button to toggle the number's sign to make it
    negative
  • LONG PRESS the ALARM button to reset the number to 0
  • Press MODE to proceed to selecting the operator

Selecting the operator:

  • Press the LIGHT button to cycle through available operators. They are:
    + Add
    - Subtract
    * Multiply
    / Divide
    sqrtf() Square root
    powf() Power (exponent calculation)
  • Press MODE or ALARM to proceed to entering the second number

Entering the second number:

  • Everything is the same as setting the first number except that pressing
    MODE here will proceed to viewing the results

Viewing the results:

  • Pressing MODE will start a new calculation with the result as the first
    number. (LONG PRESS ALARM to reset the value to 0)

Errors:

  • An error will be triggered if the result is not able to be displayed, that
    is, if the value is greater than 9,999.99 or less than -9,999.99.
  • An error will also be triggered if an impossible operation is selected,
    for instance trying to divide by 0 or get the square root of a negative
    number.
  • Exit error mode and start over with any button press.

@joeycastillo
Copy link
Owner

Playing around with this in the simulator — love it! One issue with the long press of mode: it conflicts with the ability of the wearer to quickly advance past the calculator face. Like if they had Clock -> Calculator -> Stopwatch in rotation, they'd have to remember to long press on this face.

A suggestion: could you add to your conditional for the mode button that if all the numbers are 0, and you're in calculator mode 0, a short press of mode just advances to the next watch face? This way if the wearer just need to advance through their watch faces, calculator won't stop them. It feels like a simple tradeoff at very little cost: when you're in mode 0, and all the numbers are 0, it doesn't really make sense to advance to selecting an operator since zero plus or minus any number is just that number, and zero multiplied, divided or raised to any power is still zero.

@mcguirepr89
Copy link
Contributor Author

Okay, I made the suggested changes and tested them out and agree that's much smoother. I also added back the default of mode long press going to movement_move_to_face(0). Please let me know if you have any other feedback and THANKS SO MUCH FOR SENSOR WATCH!

@mcguirepr89 mcguirepr89 deleted the add_simple_calculator_face branch August 31, 2024 16:56
@mcguirepr89 mcguirepr89 restored the add_simple_calculator_face branch August 31, 2024 16:56
@mcguirepr89
Copy link
Contributor Author

Whoops -- accidentally deleted the branch!! It's back now, though 😄

@mcguirepr89 mcguirepr89 reopened this Aug 31, 2024
@mcguirepr89
Copy link
Contributor Author

mcguirepr89 commented Aug 31, 2024

I'm pretty sure I just found a bug in this so please don't accept it yet -- my suspicion is that the sign(state->negative) is not cleared properly and is misleading the user on whether the number is negative or positive when reusing the results as fist_num -- I'll let you know what I find soon

@joeycastillo
Copy link
Owner

Two more notes that occur to me:

  1. Should the calculator zero out the operators and operands when it first comes on screen? This way the mode button advances immediately, rather than having to manually clear out an old calculation. The downside is you can't keep the result of a calculation once you leave the calculator, but I think that's probably a fair tradeoff?
  2. Maybe a long hold on the Mode button should be an "All Clear" that resets the operators and the operands to 0. This way after doing a calculation, you can long press mode to clear, then either press mode to advance, or long press mode to return to first watch face. Roughly:
case EVENT_MODE_LONG_PRESS:
    if ((state->first_num == 0) && (state->second_num == 0)) {
        movement_move_to_face(0);
    } else {
        state->mode == MODE_ENTERING_FIRST_NUM;
        state->operation = OP_ADD;
        state->first_num = 0;
        state->second_num = 0;
    }
    break;

From here, your existing MODE_BUTTON_UP code works as-is, moving to the next watch face since the conditions for doing that are now met.

@mcguirepr89
Copy link
Contributor Author

mcguirepr89 commented Aug 31, 2024

Found the bug -- negative toggle was logically only affecting the first_num, even when the second_num was being edited

@mcguirepr89
Copy link
Contributor Author

mcguirepr89 commented Aug 31, 2024

Two more notes that occur to me:

  1. Should the calculator zero out the operators and operands when it first comes on screen? This way the mode button advances immediately, rather than having to manually clear out an old calculation. The downside is you can't keep the result of a calculation once you leave the calculator, but I think that's probably a fair tradeoff?

I've tinkered with this enough to want to save the last value -- the way I use this is in calculations that I revisit -- but if you think the general user wants a clean start every time, that is the essence of the PR into main: appealing to the general user. It's also helpful if you do the calculation and then the TIMEOUT hits and you are taken back to face(0), you don't lose your results. Let me know!

  1. Maybe a long hold on the Mode button should be an "All Clear" that resets the operators and the operands to 0. This way after doing a calculation, you can long press mode to clear, then either press mode to advance, or long press mode to return to first watch face. Roughly:
case EVENT_MODE_LONG_PRESS:
    if ((state->first_num == 0) && (state->second_num == 0)) {
        movement_move_to_face(0);
    } else {
        state->mode == MODE_ENTERING_FIRST_NUM;
        state->operation = OP_ADD;
        state->first_num = 0;
        state->second_num = 0;
    }
    break;

From here, your existing MODE_BUTTON_UP code works as-is, moving to the next watch face since the conditions for doing that are now met.

Let me give this a test tomorrow after some dinner and sleep and I'll let you know 😄

Thanks again for looking at the code

@mcguirepr89
Copy link
Contributor Author

This works fine in the emulator, but it runs differently on hardware. I'm seeing results with the '-' sign when the number is positive, though the calculations are working otherwise. For instance, inputting 5 + -2 will make 3, but the negative sign '-' is displayed. I can't get this to happen in the emulator, though. I'm not sure what could be going on?

@mcguirepr89
Copy link
Contributor Author

Okay, I finally squashed that bug -- I forgot to explicitly make the negative member false by default. This is working on hardware now.

@matheusmoreira matheusmoreira self-assigned this Sep 7, 2024
@matheusmoreira matheusmoreira added enhancement New feature or request watch-face Related to a new or existing watch face tested-on-hw This feature or pull request has been tested on a physical watch labels Sep 7, 2024
matheusmoreira added a commit that referenced this pull request Sep 7, 2024
Adds a simple calculator watch face to the sensor watch
capable of addition, subtraction, multiplication, division
and exponentiation.

Reviewed-by: Joey Castillo <[email protected]>
Reviewed-by: Matheus Afonso Martins Moreira <[email protected]>
Tested-by: Joey Castillo <[email protected]>
Tested-on-hardware-by: mcguirepr89 <[email protected]>
GitHub-Pull-Request: #454
@matheusmoreira matheusmoreira added the next This feature or pull request is present in the next branch label Sep 7, 2024
voloved pushed a commit to voloved/Sensor-Watch that referenced this pull request Sep 8, 2024
Adds a simple calculator watch face to the sensor watch
capable of addition, subtraction, multiplication, division
and exponentiation.

Reviewed-by: Joey Castillo <[email protected]>
Reviewed-by: Matheus Afonso Martins Moreira <[email protected]>
Tested-by: Joey Castillo <[email protected]>
Tested-on-hardware-by: mcguirepr89 <[email protected]>
GitHub-Pull-Request: joeycastillo#454
@voloved
Copy link
Contributor

voloved commented Sep 8, 2024

This is a small UX thing: the calculator doesn't show the display until a tick occurs.
It looks like the tick frequency is 4 HZ, so that can be <=250ms between the screen being cleared and the calculator appearing.

Below is an easy fix to this, but I only reviewed the source code for 3 minutes :)

    switch (event.event_type) {
        case EVENT_ACTIVATE:
-            break;
-
        case EVENT_TICK: 
            switch (state->mode) {
                case MODE_ENTERING_FIRST_NUM:
                    // See the WISH for this function above
                    set_number(&state->first_num, 
                            state->placeholder,
                            display_string, 
                            temp_display_string, 
                            event,
                            1);
                    break;                

                case MODE_CHOOSING:
                    set_operation(state);
                    break;

@matheusmoreira matheusmoreira merged commit 337864e into joeycastillo:main Sep 17, 2024
2 checks passed
@matheusmoreira matheusmoreira added the main This feature or pull request is present in the main branch label Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request main This feature or pull request is present in the main branch next This feature or pull request is present in the next branch tested-on-hw This feature or pull request has been tested on a physical watch watch-face Related to a new or existing watch face
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants