Calculator App

[Raupinger]

A calculator based on the Shunting-yard algorithm as described here. The basic functionality is in place but some work still needs to be done:

• Second page of buttons for braces, pow etc.
• Testing various edge cases
• Haptic feedback for invalid operations
• Clean up the UI
• Make sure things like Precedence are handled correctly

this resolves #227

[Raupinger]

Video

2588-10mB.mp4

[Raupinger]

Ok, I'm declaring this "ready for review" now. I cant find any bugs anymore and spent hours trying to reduce the memory usage. One obvious thing that could still be done is parsing the numbers while putting everything on the input stack. that would save n*(k)+31 bytes, where n is the number of digits a number has and k is the number of bytes a single digit on the stack takes up. in praxis it should be at the lower range of that. Overflows are not handled. The eval function has a lot of resemblance with Italian cuisine, so i wouldn't be surprised if there are still some errors in it. I couldn't test that the motor actually works but i don't expect any surprises there.

[Raupinger]

video

[Dudemanjude]

I've found a bug that causes the watch to reset. If you just enter a period, and then hit equals, it resets. This happens no matter how many periods you add. For example, if you enter just "." and then hit equals, it will reset, and "...." and then equals will also reset. Another case of unwanted behavior happens when you put "1+.1" into the calculator, and hit equals. It says the answer is "2.1", while I would expect it to say "1.1". If you type ".2+.3", it results in "5", which is also unexpected.

The build that I'm running on my watch is a custom one that I compiled from your Calc branch, plus a custom Terminal watchface from a different repo. I'm a beginner when it comes to Git and compiling InfiniTime, so it's possible that my build is messed up, although everything else seems to work just fine.

Otherwise, the app is great, and I hope that once these bugs get sorted out, we can have a calculator built into one of the next versions of InfiniTime!

[Raupinger]

Yeah, that sounds plausible. Currently there's no handling for periods that dont appear after some digits, so they just act as if nothing was there at all. The reset can be explained by trying to evaluate an empty expression. I should probably check for that and either interpret it as a zero or treat it as an invalid expression, vibrate and return

[Raupinger]

Thanks @jonvmey for all those reviews

[Izaic]

Is this just waiting to be merged or?

[Raupinger]

Its should be ready. Its just been long enough that I would probably have to rebase it again

[KoalaV2]

Any updates on this?

[Raupinger]

Any updates on this?

It's basically done. At the time JF was to busy to take a look at this and its just kind of been sitting here since. Doesn't help that I've not been active on Infinitime for a while now. Rebasing it doesn't look to bad assuming it doesn't run into memory limits again.

[fossdd]

i would really like that feature on InfiniTime

[Alpha-Titiwu]

I have successfully tested this nice calculator by rebasing it on the latest develop branch. It would be great, if it could be merged into the official codebase.
@Raupinger could you please resolve the merge conflicts?

[Vistaus]

This would be a handy feature indeed. 🙂

[NeroBurner]

rebased the Calc branch on current develop: https://github.com/NeroBurner/InfiniTime/tree/Calc_rebase

added the lv_simulation branch on top: https://github.com/NeroBurner/InfiniTime/tree/lv_simulation_Calc

Nice for local hacking:

[Raupinger]

Just a quick heads up: Ive had some incorrect results (like 0.3*0.3) and am still not super confident about my overflow checks. I would like to pull out the eval function and write some tests for it before this gets realeased.

[Raupinger]

Also, thanks for rebasing

[Vistaus]

Too bad this didn't get merged yet. I like that the main devs are active, but the main feature in 1.9.0 is a terminal watchface, really? I mean: it looks great and geeky, so I really dig it! :) But if I had to choose, I would've liked to see something that's actually useful, like this calculator app.

[medeyko]

Just a quick heads up: Ive had some incorrect results (like 0.3*0.3) and am still not super confident about my overflow checks. I would like to pull out the eval function and write some tests for it before this gets realeased.

Had you an opportunity to raise your confidence in overflow checks? If not, could you please write more details?

[JF002]

Too bad this didn't get merged yet. I like that the main devs are active, but the main feature in 1.9.0 is a terminal watchface, really? I mean: it looks great and geeky, so I really dig it! :) But if I had to choose, I would've liked to see something that's actually useful, like this calculator app.

Please note that there are a lot of reasons why any specific PR is not merged yet : some are not finished or do not meet the project goals, priorities and/or code quality, they need time and attention from developers to review it and maintain it, they need too much memory and cannot be merged until we can find a way to free some ram/flash memory space,...

As you can see, there are currently ~100 open PR, and I'm really sorry I cannot give them the attention they deserve, but I (along with other core developers) work on this project on our free time, and we cannot possibly review and merge that much PR as soon as they are opened.
Also, remember that the PineTime is a very constrained hardware (in terms of processing power and memory). In the end, it'll be impossible to merge each and every features without hitting processing or memory limitations, so choices have to be made, unfortunately.

I'm sorry you don't find the terminal watchface as amazing as I do, but I'm pretty sure that If we merged this one instead, other people would have said that they don't need a calculator, and that they are sad the new watchface is not released yet.
Also, note that the terminal watchface is not the only addition in 1.9 : 49 PR and 100 commits were merged since 1.8. Maintaining software do not only consist in adding feature visible to the end user. It also consist in fixing bugs, improving the code that is running under the hood, improving performances and maintainability of the code,...

Now, regarding this PR, the first question I'll ask before going any further is : what is the added memory usage (flash and ram ) of this PR ?

[Danimations]

Just writing to express my support for this feature. I would definitely use it once it's available.

[NeroBurner]

rebased the Calc branch on current develop: https://github.com/NeroBurner/InfiniTime/tree/Calc_rebase

Additionally I refactored the PR into three single commits to make rebases easier when the fonts were changed by other PRs:

• fonts/README: add calculator symbol to font generation documentation
• fonts: regenerate jetbrains_mono_bold_20 with calc symbol
• Add Calculator screen as new App (the changes from @Raupinger )

I propose to replace the current branch content with the new Calc_rebase commits. To do that you can do the following steps (assuming origin is [email protected]:Raupinger/Pinetime.git)

git remote add neroburner https://github.com/NeroBurner/InfiniTime.git
git fetch neroburner Calc_rebase
git switch Calc
git reset --hard neroburner/Calc_rebase
git push --force-with-lease origin Calc

[Raupinger]

Thanks!

[rev22]

I tried to reduce the firmware size and ram usage by not using heap-allocated stacks, and removing the intermediate tree data structures, without changing the algorithm.

Some issues were solved, for example with displaying the result of 0.3x0.3, and with expressions like -(2^32)+1

Feel free to cherry-pick the commit: rev22@d1bedeb (updated)

It's based on NeroBurner's rebase above.

The binary size of the calculator changed from 13164 to 8628 bytes in my test builds with gcc-arm-11.2-2022.02 and -O2 -Os. I suppose a large part of it is the lv_btnmatrix component

[ItzSwirlz]

Any status on this besides a potential 1.10 rebase if needed? I'd love to see this merged

[MaWalla]

I merged the rebase from @NeroBurner into the current develop state and resolved all conflicts. Hope I did it right! 😄

https://github.com/MaWalla/InfiniTime/tree/Calc_rebase

[omginput]

Could someone please tell why this hasn't been merged yet?

[JustScott]

I updated the code to work with v1.14, its available on my branch: https://github.com/JustScott/InfiniTime/tree/calculator_app.

While it's working in InfiniSim, I get the following error attempting to build the mcuboot-app directly:

/Pinetime/gcc-arm-11.2-2022.02-x86_64-arm-none-eabi/bin/../lib/gcc/arm-none-eabi/11.2.1/../../../../arm-none-eabi/bin/ld: region RAM overflowed with stack Memory region

I'm assuming this means the Calculator.cpp code needs some memory optimizations before the app can run on the Pinetime's hardware...which is a bit above my skill level at the moment, so if anyone would like to try this out feel free to use my branch!

[julianfoad]

Memory Optimisations

@JustScott re. "needs some memory optimizations", this commit solves it for me: https://lab.trax.im/gentle/infinitime/-/commit/eebd027eecb2a4c756d02efecd11f96fc914d95a

Calculator: move 160 bytes from RAM to flash

Making the 'button matrix' arrays 'const' saves 160 bytes of non-stack
RAM, which in this context is a lot, and makes it fit (for me) in a
real-device build along with my choice of other apps.

The essence of the change is:

@@ -124,3 +124,3 @@ Calculator::~Calculator() {
 
-static const char* buttonMap1[] = {
+static const char* const buttonMap1[] = {
   "7", "8", "9", "/", "\n",
@@ -131,3 +131,3 @@ static const char* buttonMap1[] = {
 
-static const char* buttonMap2[] = {
+static const char* const buttonMap2[] = {
   "7", "8", "9", "(", "\n",

It also includes three const_casts which are needed to overcome what appears to be an oversight in LVGL's const-correctness, which I have verified to be a safe casting.

You can pull that specific commit from my repo into your calculator_app branch like this (as I choose not to do my work in Microsoft Github):

git pull https://lab.trax.im/gentle/infinitime/ eebd027eecb2a4c756d02efecd11f96fc914d95a

or pull my calculator_app branch (which is based on yours) like this:

git pull https://lab.trax.im/gentle/infinitime/ calculator_app

which is currently just that single commit but I might add more.

Let me know if it works for you, if you try it. Thanks.

[tituscmd]

Anything holding this back from being merged?

[julianfoad]

@tituscmd yes of course there is. please read the comments especially #375 (comment)

[tituscmd]

That comment being 3 years old, I thought I might as well check up on it again. My bad!

[mark9064]

There's also #1483 to consider