WP 34S and 31S bugs and fixes

[Nigel (UK)]

(12-27-2014 02:33 PM)Marcus von Cube Wrote:  

(12-26-2014 06:12 PM)Nigel (UK) Wrote:  I've given a WP-34S containing firmware with the question mark problem to someone as a Christmas present. Apart from the unnecessary menu entry, are there likely to be any more problems with it?

I'd bet there are. It is better to update the device if possible.

Here's how the story finally ends.

Your update to the makefile didn't fix my particular problem - I had flashed my calculator with a file from the repository that didn't have the issue and mistakenly thought that it had gone for ever. I finally tracked down the cause to the compiler - not the yagarto compiler, but the gcc 4.8.1 compiler that came with MinGW. The flag -DREALBUILD as a compiler option was generally not defining REALBUILD, so that the INCLUDE_STOPWATCH "flag" was set in features.h (and hence elsewhere) in some runs and not set in other runs. This led to variations in the number of functions defined in different compiler runs, one effect of which was the "gap" in the catalogue that I noticed. I am sure that there were other effects as well, none of them good!

I have now reverted to an earlier gcc compiler (4.5.2) and everything compiles without issue. I shall not update MinGW again!

Thank you for helping me with this - the correct copy of catalogues.h was invaluable.

Nigel (UK)

[Bit]

(12-23-2014 01:38 PM)Bit Wrote:  

(12-23-2014 12:02 PM)PANAMATIK Wrote:  IMO a logical integer contraction would be: mantissa, then 'e' followed by the decimal number of digits that are not shown. i.e. in binary 10011011 -> 1001e04 decimal 1234567 -> 1234e3. Its like a non normalized exponential display in real mode and easy to understand. 'e' is for: exponent in the current base, expressed as decimal number.

The little bars can still be shown, there is a maximum of 8 bars used in binary mode at wordsize 64, and there are enough matrix columns left when you show 3 digits of mantissa. The not used decimal point gives you another 3 columns.

Bernhard

I'm a little hesitant about using the exponent notation for integers but it might just fit my description of "a more or less reasonable way it should work". Even if I decide to, I won't be able to work on this for a while right now. Would anyone else like to share their thoughts regarding how good of a user interface the above may be?

I've created an implementation and I have to admit it seems to be a better user interface than I suspected it would be. Bernhard, thank you for the idea. Please see this post in the Y register display thread for the details.

[Bit]

If an error occurred on the 34S after an operation that didn't disable stack lift, and tracing was also enabled, the stack would be lifted twice, duplicating the X register.

Here's how to reproduce the problem in single precision mode with flag D cleared:
[EEX] [1] [5] [0] [ENTER] [*] [EEX] [9] [9] [*]
Simpler way that also works in double precision mode:
[1] [ENTER] [+] [0] [/]

I've committed a fix. The problem was that xeq() saved the contents of the original stack after it called print_trace(), which in turn calls process_cmdline() and possibly lifts the stack.

[Bit]

The YDON and YDOFF commands were included in the 34S and 31S even if Y register display was completely disabled at compile time. I've fixed that and also added an option to permanently enable Y register display, which saves 284 bytes in the 34S so it may be useful in some cases.

[walter b]

(01-08-2015 04:34 AM)Bit Wrote:  The YDON and YDOFF commands were included in the 34S and 31S even if Y register display was completely disabled at compile time. I've fixed that ...

Thanks a lot for your work in bug hunting. Much appreciated.

d:-)

[Bit]

(01-08-2015 04:48 AM)walter b Wrote:  

(01-08-2015 04:34 AM)Bit Wrote:  The YDON and YDOFF commands were included in the 34S and 31S even if Y register display was completely disabled at compile time. I've fixed that ...

Thanks a lot for your work in bug hunting. Much appreciated.

d:-)

You're welcome.

There's more:

1. Incomplete or invalid fractions cause delayed error messages
   If you type in an incomplete fraction (a ^b/_nothing) or an invalid fraction (a ^b/₀) and then enter a catalog or perform another operation that immediately changes the display, the error message caused by the bad fraction will appear after the operation that follows the catalog (or similar) activity.
   
   Here's how to reproduce the problem:
   [1] [.] [2] [.] [h] [MODE] [EXIT] [+] -> Domain error
   Funnier:
   [1] [.] [2] [.] [h] [MODE] [EXIT] [1] -> Domain error
   Also:
   [1] [.] [2] [.] [h] [P/R] (program away...) [EXIT] [R/S] -> Domain error
   And so on. If double-dot fractions (a..b -> a/b) are enabled, they have the same problem.
   
   I've created a patch that produces similar behavior as that of the 35S: You get an error message immediately if you try to perform any operation with an invalid command line. (The only exception is turning off the calculator, which always succeeds.)
   
   
2. CLx produces domain error if the command line contains a bad fraction
   CLx always clears the command line but still produces a domain error if an incomplete or invalid fraction was entered. This makes little sense in my opinion since the bad input that CLx complains about was never used. I suggest that CLx silently clear the command line.
   
   
3. EEX not allowed in fractions mode even if no fraction was entered
   The specials() function in xeq.c disables EEX in integer modes or whenever a fraction is being entered, which makes sense. However, it also disables EEX in fractions mode even if no fraction was entered. That looks to me like an oversight (I couldn't find anything about it in the documentation) and I suggest we allow exponent entry in fractions mode as long as no fraction is being entered (the 35S does the same).
   

I have patches ready for all three issues.

Walter, could you please comment on whether you think my proposed fixes should be committed, or if you'd like some other behavior?

[walter b]

(01-08-2015 05:28 AM)Bit Wrote:  

1. Incomplete or invalid fractions cause delayed error messages ...
2. CLx produces domain error if the command line contains a bad fraction ...
3. EEX not allowed in fractions mode even if no fraction was entered ...
   

I have patches ready for all three issues.

Walter, could you please comment on whether you think my proposed fixes should be committed, or if you'd like some other behavior?

For sure

3. is done on purpose. Fractions on the WP 34S have a very limited range. Well, it's larger than on the HP-33Sii but anyway EEX doesn't make real sense here. So please leave that as it is.

I agree with you on 2.

Your suggestion concerning 1. sounds reasonable but I need some time to check. Please be patient here, I think I can do it this evening.

d:-)

[walter b]

(01-08-2015 07:09 AM)walter b Wrote:  

(01-08-2015 05:28 AM)Bit Wrote:  

1. Incomplete or invalid fractions cause delayed error messages ...
2. CLx produces domain error if the command line contains a bad fraction ...
3. EEX not allowed in fractions mode even if no fraction was entered ...
   

I have patches ready for all three issues.

Walter, could you please comment on whether you think my proposed fixes should be committed, or if you'd like some other behavior?

...

Your suggestion concerning 1. sounds reasonable but I need some time to check. Please be patient here, I think I can do it this evening.

Ok, I also agree on your suggestion for 1.

d:-)

[Bit]

(01-08-2015 04:38 PM)walter b Wrote:  

(01-08-2015 07:09 AM)walter b Wrote:  

(01-08-2015 05:28 AM)Bit Wrote:  

1. Incomplete or invalid fractions cause delayed error messages ...
   

Your suggestion concerning 1. sounds reasonable but I need some time to check. Please be patient here, I think I can do it this evening.

Ok, I also agree on your suggestion for 1.

d:-)

My original patch worked in such a way (because it was simpler to implement) that whenever the command line contained an invalid fraction, a domain error would be produced when you pressed the button for any operation, not when the button was released. Thus the names of operations wouldn't be displayed and they couldn't be canceled (NULL-ed). While that's an improvement, it's not the most elegant solution because it isn't consistent with how the calculator usually behaves.

I created another patch (it needs a bit more testing) that's better IMO: Error messages are only produced when you try to execute an operation by releasing the button, but not if you cancel it. (Keys that immediately do something like entering a catalog still produce an error message on key press.) The cost is roughly 50 bytes in the firmware because the function that verifies the command line needs to be invoked from more places.

Which behavior should be adopted?

[Bit]

(01-08-2015 07:09 AM)walter b Wrote:  

(01-08-2015 05:28 AM)Bit Wrote:  

1. Incomplete or invalid fractions cause delayed error messages ...
2. CLx produces domain error if the command line contains a bad fraction ...
3. EEX not allowed in fractions mode even if no fraction was entered ...
   

...
I agree with you on 2.
...

Should CLx silently erase the command line only when invoked directly from the keyboard or also when it's executed in a program?

[Paul Dale]

I'd say CLx should silently clear the command line in both.


Pauli

[walter b]

(01-09-2015 12:38 AM)Bit Wrote:  Should CLx silently erase the command line only when invoked directly from the keyboard or also when it's executed in a program?

It should do the same in both modes IMO. But it's 3 a.m. here so I might have overlooked some side effects.

d:-O

[walter b]

(01-09-2015 12:32 AM)Bit Wrote:  I created another patch (it needs a bit more testing) that's better IMO: Error messages are only produced when you try to execute an operation by releasing the button, but not if you cancel it. (Keys that immediately do something like entering a catalog still produce an error message on key press.) The cost is roughly 50 bytes in the firmware because the function that verifies the command line needs to be invoked from more places.

Which behavior should be adopted?

I'd vote for your new patch.

d#-O

[Bit]

(01-09-2015 12:38 AM)Bit Wrote:  

(01-08-2015 07:09 AM)walter b Wrote:  ...
I agree with you on 2.
...

Should CLx silently erase the command line only when invoked directly from the keyboard or also when it's executed in a program?

CLALL, CLSTK and RESET also effectively discard the command line. They're only accessible from the keyboard via catalogs so the command line will always be processed before they execute. But CLSTK can also be programmed and the following program produces a domain error:
[1] [.] [2] [.] [CLSTK]

This isn't a big problem in my opinion but fixing it in addition to CLx doesn't further increase the size of the firmware, so I suggest we do the right thing here and fix CLSTK as well.

[walter b]

(01-10-2015 06:28 PM)Bit Wrote:  

(01-09-2015 12:38 AM)Bit Wrote:  Should CLx silently erase the command line only when invoked directly from the keyboard or also when it's executed in a program?

... CLSTK can also be programmed and the following program produces a domain error:
[1] [.] [2] [.] [CLSTK]

This isn't a big problem in my opinion but fixing it in addition to CLx doesn't further increase the size of the firmware, so I suggest we do the right thing here and fix CLSTK as well.

Agreed.

d:-)

[Bit]

(01-08-2015 05:28 AM)Bit Wrote:  There's more:

1. Incomplete or invalid fractions cause delayed error messages ...
2. CLx produces domain error if the command line contains a bad fraction ...
3. EEX not allowed in fractions mode even if no fraction was entered ...
   

I've committed patches that fix issues 1 and 2.

I've also added a compile time option (IGNORE_INVALID_FRACTIONS) that disables command line checks and causes the calculator to simply ignore invalid fractions: a.b.0 becomes the integer a and a..0 (if such fractions are enabled) becomes zero. It saves ~ 252 bytes in the 34S, and considering that invalid fractions never appear automatically or in any non-obvious way, some may prefer not to spend that much firmware space only to report that one error condition.

Please test and report any issues or weird behavior.

[DMaier]

I think this is a bug...

The product (Pi) and sum (Sigma) functions don't behave as described in the IOP in the V3.2 printed manual. Specifically, the IOP indicates that they are both one number functions. However, they also fill the stack with zeroes on return.

From

Code:

[r3723]: trunk / xrom / prodsum.wp34s:
sum_product_okay::    RCL .03
            STO L
            Num 0
            FILL
            RCL .01
            RTN

Perhaps this is just a documentation issue. However, I think most users would expect the stack to be preserved.

[Paul Dale]

(01-11-2015 06:51 PM)DMaier Wrote:  I think this is a bug...

They have never preserved the stack -- they've always behaved like solve/integrate and taken over and replaced the stack. A price to pay for calling a users' code basically.


Pauli

[Bit]

(01-11-2015 09:40 PM)Paul Dale Wrote:  

(01-11-2015 06:51 PM)DMaier Wrote:  I think this is a bug...

They have never preserved the stack -- they've always behaved like solve/integrate and taken over and replaced the stack. A price to pay for calling a users' code basically.


Pauli

This wouldn't be difficult to change if that's desired, see the patch below. Solve and integrate could probably be adapted in a similar way. (STOS/RCLS aren't currently used by any XROM routines so it's safe to change their behavior when called from XROM.)

Code:

diff -ur wp34s_r3723/xeq.c wp34s_sum_prod_save_stack_r3723_20150111/xeq.c
--- wp34s_r3723/xeq.c    2015-01-10 23:35:52.000000000 -0500
+++ wp34s_sum_prod_save_stack_r3723_20150111/xeq.c    2015-01-11 19:44:51.956524634 -0500
@@ -1859,7 +1859,8 @@
 
 /* Save and restore the entire stack to sequential registers */
 static int check_stack_overlap(unsigned int arg) {
-    const int n = stack_size();
+    // Save or restore full user stack even if called from XROM.
+    const int n = (UState.stack_depth ? 8 : 4);
 
     if (arg + n <= global_regs() || arg >= NUMREG) {
         return n;
diff -ur wp34s_r3723/xrom/prodsum.wp34s wp34s_sum_prod_save_stack_r3723_20150111/xrom/prodsum.wp34s
--- wp34s_r3723/xrom/prodsum.wp34s    2014-11-15 13:18:44.000000000 -0500
+++ wp34s_sum_prod_save_stack_r3723_20150111/xrom/prodsum.wp34s    2015-01-11 19:46:48.944521816 -0500
@@ -32,8 +32,8 @@
         XLBL"SIGMA"                /* Entry: SUMMATION */
             INTM?
                 ERR ERR_BAD_MODE
-            LocR 04                /* Registers .00 to .03 */
-            STO .03                /* Save for LastX*/
+            LocR 11                /* Registers .00 to .10 */
+            STOS .03            /* Save stack */
             SPEC?
                 JMP sum_product_nan
             STO .00
@@ -71,8 +71,8 @@
         XLBL"PRODUCT"                /* Entry: PRODUCT */
             INTM?
                 ERR ERR_BAD_MODE
-            LocR 04                /* Registers .00 to .03 */
-            STO .03
+            LocR 11                /* Registers .00 to .10 */
+            STOS .03            /* Save stack */
             SPEC?
                 JMP sum_product_nan
             STO .00
@@ -95,11 +95,9 @@
 product_entry::        DSL .00
                 JMP product_loop
 
-sum_product_okay::    RCL .03
+sum_product_okay::    RCLS .03
             STO L
-            Num 0
-            FILL
-            RCL .01
+            x[<->] .01
             RTN
 
 sum_product_nan::    Num NaN

[Paul Dale]

(01-12-2015 12:48 AM)Bit Wrote:  This wouldn't be difficult to change if that's desired, see the patch below. Solve and integrate could probably be adapted in a similar way. (STOS/RCLS aren't currently used by any XROM routines so it's safe to change their behavior when called from XROM.)

For consistency, f' and f" should also do the same and likely solve and integrate.

Should the stack or some portion of it be preserved when passed to the user's routine? What if that routine makes changes? Currently, A .. D aren't changed and aren't filled by these and the user can do what they want with them even if they are in the stack.


- Pauli