InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.75k stars 941 forks source link

Simple calculator #1483

Open minacode opened 1 year ago

minacode commented 1 year ago

This PR implements a simple fixed-point calculator. In contrast to #375 it aims to be simpler by not parsing whole equations but just evaluating one operation at a time over an accumulated result.

It takes an operation and a value (digit-wise) and applies the value to the result with the chosen operation whenever a new operation is chosen or = is pressed.

=: evaluate current input <: removes the input charwise or reset the result if the input is zero. (-): toggle the sign of the current input (thanks @FintasticMan) + - / * /: press the button to cycle through the operators and "no-operator".

It uses fixed-point numbers and checks for errors like too big results and zero division.

Known issues:

from @yusufmte's comment below:

minacode commented 1 year ago

Thank you very much for the review! I hope to find the time soon to fix these bugs.

minacode commented 11 months ago

I tried to update to the new interface for optional apps, but can't fix the overflowing RAM. If someone knows how to fix this, please write a comment.

minacode commented 9 months ago

I updated this to the new CMake optional apps interface and tested it on the device. I think it is ready to merge.

mark9064 commented 6 months ago

Have been running this app for a while now, works great. Few minor code comments as above (but I haven't reviewed the whole thing in depth, just scanned through it)

minacode commented 2 months ago

This comment in the other calculator implementation is interesting. Saving it here for later.

minacode commented 2 months ago

Turns out that adding this const to the button matrix does indeed change something, because we no longer need to reduce the FreeRTOS heap size.

Runs well on my watch for a day now.

mark9064 commented 2 months ago

Nice! Is constexpr possible or more correct here? I think it's preferred way when it's possible but I'm honestly not entirely sure...

minacode commented 2 months ago

I don't know, but I will try. What exactly? The first const? Both? 😄

mark9064 commented 2 months ago

I think constexpr const char* should suffice? AFAIK this means compile time evaluate this constant list of const char* (i.e constant strings)

minacode commented 2 months ago

The last commit did not change something, so I guess that was all we will get.

mark9064 commented 2 months ago

Oh they're equivalent, it's that constexpr is nicer to use in general as it is a guarantee of compile time evaluation (if it cannot be evaluated at compile time compilation fails). The way you had previously is the way to do it in C, with constexpr added to C++ constexpr const char* is the more modern way from what I understand

minacode commented 2 months ago

So we just leave it like this, because more const -> more good? :grinning:

mark9064 commented 2 months ago

The const immediately after constexpr is redundant (constexpr implies const) so I'd remove that personally but it doesn't make a difference in terms of semantics