WP31s error recovery

[Dieter]

I just discovered that the 31s behaves differently from other HPs or the WP34s after an error occured. Example: type 3 [ENTER] -4 [LN] and you'll get the expected "Domain Error". Usually pressing backspace clears the error message and returns to the previously entered –4. The 31s however has lost this value and shows the 3 that was previously in Y. In fact, now both X and Y (!) hold this value.

I thought that the UNDO feature may help here, but pressing UNDO after the error occured does not recover the –4 either. Neither on the emulator nor on real hardware.

Is this the intended behaviour? Sorry if this has been discussed earlier, in this case a pointer to the respective thread will do. ;-)

Dieter

[BarryMead]

(03-01-2015 10:07 PM)Dieter Wrote:  Is this the intended behaviour? Sorry if this has been discussed earlier, in this case a pointer to the respective thread will do. ;-)
Dieter

According to page 26 of the PDF manual this is NOT the intended behavior. The Back Arrow key should clear the error message and return the display to the state before the error occurred.

I don't recall ever seeing this subject reported in the forums, but I must admit that I typically don't read as many WP-31s threads as I do WP-34s threads. It looks to me like you found another genuine bug.

[Sanjeev Visvanatha]

The intended behaviour for both Back Arrow and Undo is correctly described in the manual. What you found appears to be a bug.

[MarkHaysHarris777]

(03-02-2015 04:18 AM)Sanjeev Visvanatha Wrote:  The intended behaviour for both Back Arrow and Undo is correctly described in the manual. What you found appears to be a bug.

Which build did you use?

marcus

[Dieter]

(03-02-2015 06:05 AM)MarkHaysHarris777 Wrote:  Which build did you use?

I don't know which build Sanjeev uses, but the described behaviour occurs both on the emulator with version 1.2 3742 and the "real thing" with version 1.2 3754.

The problem obviously affects the [←] key as well as UNDO.

Example:

Code:

1 [ENTER] 2 [ENTER] 3 [ENTER] 4

T   1        1        1
Z   2        1        2
Y   3        2        3
X   4        7        3   (!)
       [+]     [UNDO]

On the other hand page 26 of the preliminary 31s PDF manual states that UNDO "...will restore the stack and all other registers exactly as they were before the last command was executed".

Dieter

[BarryMead]

(03-02-2015 06:42 AM)Dieter Wrote:  Example:

Code:

1 [ENTER] 2 [ENTER] 3 [ENTER] 4

T   1        1        1
Z   2        1        2
Y   3        2        3
X   4        7        3   (!)
       [+]     [UNDO]

In the example you give above, the "Stack" does get restored by the UNDO command. The number 4 is not actually on the stack when the + key is pressed. It is in the "Input" register not the X register. If you do this sequence:

Code:

 1 [ENTER] 2 [ENTER] 3 [ENTER] 4 [X<>Y] [X<>Y]

T  1        1        1
Z  2        1        2
Y  3        2        3
X  4        7        4
       [+]    [UNDO]

You can see that the UNDO really does restore the actual "STACK" but in your example the number 4 was
not yet in the stack (only in the input register) so the UNDO couldn't restore it. In your example the actual X register
still had the 3 in it from [ENTER] duplication, and the input register (displayed in the lower line of the calculator) does not show
what is in the X register It shows the partially completed input register (4). If you run the QT emulator for the WP-31s you can see this in real time.

Modifying your original error report if you type: 3 [ENTER] 4 [+/-] [X<>Y] [X<>Y] [LN] [BACKSPACE] The -4 will be restored to the X register. But there is a difference in the behavior of the WP-34s and the WP-31s as follows:

WP-34s 3 [ENTER] 4 [+/-] [LN] [BACKSPACE] Shows -4 in X and 3 in Y
WP-31s 3 [ENTER] 4 [+/-] [LN] [BACKSPACE] Shows 3 in X and 3 in Y

So the difference is that on the WP-34s LN copies the input register to the X register first then computes the natural log so the backspace can restore the -4 but on the WP-31s this copy is not happening, or the error recovery (backspace) is restoring back too far, so what gets restored is the duplicate 3 from the previous [ENTER].

[Sanjeev Visvanatha]

(03-02-2015 06:05 AM)MarkHaysHarris777 Wrote:  Which build did you use?

My build must be old by now. It's 3680.

[Bit]

I noticed a while ago that there were multiple things wrong IMO with the undo function in the 31S but I haven't yet gotten around to dealing with it properly, or even figuring out whether or not the behavior matches the official documentation. This is something I'll definitely fix unless someone else does it first.

You may want to try the following patch but be warned, it hasn't yet received much testing.

Code:

diff -ur wp31s_r3756/keys.c wp31s_fix_undo_r3756_20150301/keys.c
--- wp31s_r3756/keys.c  2015-02-07 13:09:33.000000000 -0500
+++ wp31s_fix_undo_r3756_20150301/keys.c        2015-03-01 23:10:38.752104407 -0500
@@ -1964,6 +1964,7 @@
                        OpCode = 0;
 
                        if (c == (OP_NIL | OP_OFF) || !is_bad_cmdline()) {
+                               process_cmdline();
                                xcopy(&Undo2State, &UndoState, sizeof(TPersistentRam));
                                xcopy(&UndoState, &PersistentRam, sizeof(TPersistentRam));
                                xeq(c);
@@ -2028,19 +2029,27 @@
                        break;
 
                case STATE_UNDO:
-                       xcopy(&PersistentRam, &UndoState, sizeof(TPersistentRam));
+                       if (CmdLineLength)
+                               CmdLineLength = CmdLineEex = CmdLineDot = 0;
+                       else {
+                               xcopy(&PersistentRam, &UndoState, sizeof(TPersistentRam));
+//                             xcopy(&UndoState, &Undo2State, sizeof(TPersistentRam));
+                       }
                        break;
 
                default:
                        if (c >= (OP_SPEC | OP_ENTER) && c <= (OP_SPEC | OP_F)) {
                                if (c != (OP_SPEC | OP_ENTER) || !is_bad_cmdline()) {
                                        // Data entry key
-                                       xcopy(&Undo2State, &UndoState, sizeof(TPersistentRam));
-                                       xcopy(&UndoState, &PersistentRam, sizeof(TPersistentRam));
+                                       cmdline_empty = (CmdLineLength == 0);
+                                       if (c == (OP_SPEC | OP_ENTER) || (CmdLineLength == 0 && c == (OP_SPEC | OP_CLX))) {
+                                               process_cmdline();
+                                               xcopy(&Undo2State, &UndoState, sizeof(TPersistentRam));
+                                               xcopy(&UndoState, &PersistentRam, sizeof(TPersistentRam));
+                                       }
 #ifndef CONSOLE
                                        WasDataEntry = 1;
 #endif
-                                       cmdline_empty = (CmdLineLength == 0);
                                        xeq(c);
                                        cmdline_empty |= (CmdLineLength == 0);
                                }

[Dieter]

(03-02-2015 07:57 AM)BarryMead Wrote:  You can see that the UNDO really does restore the actual "STACK" but in your example the number 4 was not yet in the stack (only in the input register) so the UNDO couldn't restore it. In your example the actual X register still had the 3 in it from [ENTER] duplication, and the input register (displayed in the lower line of the calculator) does not show what is in the X register It shows the partially completed input register (4). If you run the QT emulator for the WP-31s you can see this in real time.

Barry, that's a very interesting observation. However I do not think this is the way the 31s should behave. It is an ultimately "classic" RPN device, and the documentation does not know of an input register as opposed to the usual stack register X. In the first example, the [ln] key computes the natural logarithm of X, so there must have been a –4 in X before. Likewise the [+] key adds Y and X, so again there must have been a 4 in X. And finally there is a LastX command (OK, RCL L in this case) that restores the X-value before the last operation.

So I do not think that the examples show the behaviour intended by the 31s team. Pauli? Walter?

Dieter

[BarryMead]

(03-02-2015 07:34 PM)Dieter Wrote:  Barry, that's a very interesting observation. However I do not think this is the way the 31s should behave. It is an ultimately "classic" RPN device, and the documentation does not know of an input register as opposed to the usual stack register X. In the first example, the [ln] key computes the natural logarithm of X, so there must have been a –4 in X before. Likewise the [+] key adds Y and X, so again there must have been a 4 in X. And finally there is a LastX command (OK, RCL L in this case) that restores the X-value before the last operation.

So I do not think that the examples show the behaviour intended by the 31s team. Pauli? Walter?

Dieter

I agree. One would naturally tend to think that the input register and the X register are the same thing. The fact that they are different is a convenience to the developers, not the user. I applied Bit's patch and it does seem to fix the error recovery and UNDO behaviors for both of your examples. Thank You Bit! Dieter: If you would like to try out Bit's patch unzip and flash your WP-31s with this file, Barry (See a later posting in this thread for a more updated wp31s flash image with Bit's Better Undo feature).

[Dieter]

(03-02-2015 07:44 PM)BarryMead Wrote:  I applied Bit's patch and it does seem to fix the error recovery and UNDO behaviors for both of your examples. Thank You Bit! Dieter: If you would like to try out Bit's patch unzip and flash your WP-31s with this file, Barry

Thank you, Barry. I just tried that build and indeed the 31s now behaves as expected. This fix should be included in the "official" firmware release. For those who want to try the posted version: it also includes some other of Bit's modifications, for instance the SIG display mode.

BTW, in the meantime I received an "official confirmation" saying that the current implementation does not work the way it is supposed to. ;-)

Dieter

[Bit]

(03-03-2015 07:10 AM)Dieter Wrote:  Thank you, Barry. I just tried that build and indeed the 31s now behaves as expected. This fix should be included in the "official" firmware release.

I can commit the fix to SVN but I'd rather wait a little until it's been more thoroughly tested, and someone should confirm that this particular new behavior is desired for the official 31S.

[rprosperi]

(03-03-2015 01:54 PM)Bit Wrote:  I can commit the fix to SVN but I'd rather wait a little until it's been more thoroughly tested, and someone should confirm that this particular new behavior is desired for the official 31S.

Good idea to wait a bit (get it? ). Walter is away for a couple days, and best if he verifies w/the docs, etc. It's odd that it took this long for such an issue to be discovered. Can you tell if the code with the issue is original 31S release level?

[Dieter]

(03-03-2015 01:54 PM)Bit Wrote:  I can commit the fix to SVN but I'd rather wait a little until it's been more thoroughly tested

Apropos... I noticed a tiny dot in the upper display line. With YDON it's on the left border, with YDOFF above the 6 if Pi is in X. So it looks this tiny dot has some meaning. Or is it just a glitch in the software ?-)

Dieter

[BarryMead]

(03-03-2015 07:34 PM)Dieter Wrote:  

(03-03-2015 01:54 PM)Bit Wrote:  I can commit the fix to SVN but I'd rather wait a little until it's been more thoroughly tested

Apropos... I noticed a tiny dot in the upper display line. With YDON it's on the left border, with YDOFF above the 6 if Pi is in X. So it looks this tiny dot has some meaning. Or is it just a glitch in the software ?-)

Dieter

Dieter: That dot is the "Stack Size Indicator" with stack size of 4 you get one dot, with stack size 8 you get two dots (looks like a colon). I compiled the source with ALL of Bit's patches and this is one of his added features.

[Dieter]

(03-03-2015 08:10 PM)BarryMead Wrote:  Dieter: That dot is the "Stack Size Indicator" with stack size of 4 you get one dot, with stack size 8 you get two dots (looks like a colon).

I see. Not a bad idea, but I would prefer having this indicator always on the same spot, preferably on the far left as it is now in YDON mode.

(03-03-2015 08:10 PM)BarryMead Wrote:  I compiled the source with ALL of Bit's patches and this is one of his added features.

Yes, compared to the "official" releases there is a substantial number of differences. Even the function set seems to be different. Do you know if all this is documented somewhere? At first sight I thought this dot was a dust particle. But then it disappeared as I turned the calculator off. ;-)

Dieter

[BarryMead]

(03-03-2015 08:48 PM)Dieter Wrote:  Do you know if all this is documented somewhere?

Yes look at this thread here.
It documents all of Bit's patches.

[Dieter]

(03-03-2015 08:57 PM)BarryMead Wrote:  Yes look at this thread here.
It documents all of Bit's patches.

Thank you. I see there are more changes than I would have expected.

Dieter

[Jonathan Cameron]

(03-03-2015 09:05 PM)BarryMead Wrote:  I just discovered a new bug in the WP-31s Qt emulator.

If you have pi in the X register and click on the lower yellow portion of the "f" shift key (not the upper black portion), then the number changes from pi to .05488615081.

As a matter of fact the lower yellow portion of most keys do strange things not representative of what the
upper portion of the key indicates.

I suspect that this is a remnant of the behavior of the WP-34s Qt emulator which allows you to directly
click on the lower green part of the key as a shortcut to the h shift key.

But these lower key values are not properly defined for the WP-31s Qt Emulator (at least not yet).

You are probably right. I helped a little bit with some of the Qt code and I do not recall that feature at all. So It would not be surprising if it got overlooked in the conversion to the 31s.

-Jonathan

[Bit]

Even better: [f] [UNDO] switches between two saved states, or in other words, you can undo the undo operation. This patch also takes care of saving the undo state in the emulators.

Code:

diff -ur wp31s_r3756/keys.c wp31s_fix_undo_r3756_20150303/keys.c
--- wp31s_r3756/keys.c  2015-02-07 13:09:33.000000000 -0500
+++ wp31s_fix_undo_r3756_20150303/keys.c        2015-03-03 19:46:28.440076188 -0500
@@ -1964,6 +1964,7 @@
                        OpCode = 0;
 
                        if (c == (OP_NIL | OP_OFF) || !is_bad_cmdline()) {
+                               process_cmdline();
                                xcopy(&Undo2State, &UndoState, sizeof(TPersistentRam));
                                xcopy(&UndoState, &PersistentRam, sizeof(TPersistentRam));
                                xeq(c);
@@ -2028,19 +2029,28 @@
                        break;
 
                case STATE_UNDO:
-                       xcopy(&PersistentRam, &UndoState, sizeof(TPersistentRam));
+                       if (CmdLineLength)
+                               CmdLineLength = CmdLineEex = CmdLineDot = 0;
+                       else {
+                               xcopy(&Undo2State, &PersistentRam, sizeof(TPersistentRam));
+                               xcopy(&PersistentRam, &UndoState, sizeof(TPersistentRam));
+                               xcopy(&UndoState, &Undo2State, sizeof(TPersistentRam));
+                       }
                        break;
 
                default:
                        if (c >= (OP_SPEC | OP_ENTER) && c <= (OP_SPEC | OP_F)) {
                                if (c != (OP_SPEC | OP_ENTER) || !is_bad_cmdline()) {
                                        // Data entry key
-                                       xcopy(&Undo2State, &UndoState, sizeof(TPersistentRam));
-                                       xcopy(&UndoState, &PersistentRam, sizeof(TPersistentRam));
+                                       cmdline_empty = (CmdLineLength == 0);
+                                       if (c == (OP_SPEC | OP_ENTER) || (CmdLineLength == 0 && c == (OP_SPEC | OP_CLX))) {
+                                               process_cmdline();
+                                               xcopy(&Undo2State, &UndoState, sizeof(TPersistentRam));
+                                               xcopy(&UndoState, &PersistentRam, sizeof(TPersistentRam));
+                                       }
 #ifndef CONSOLE
                                        WasDataEntry = 1;
 #endif
-                                       cmdline_empty = (CmdLineLength == 0);
                                        xeq(c);
                                        cmdline_empty |= (CmdLineLength == 0);
                                }
diff -ur wp31s_r3756/QtGui/QtEmulatorAdapter.c wp31s_fix_undo_r3756_20150303/QtGui/QtEmulatorAdapter.c
--- wp31s_r3756/QtGui/QtEmulatorAdapter.c       2014-11-26 21:09:35.000000000 -0500
+++ wp31s_fix_undo_r3756_20150303/QtGui/QtEmulatorAdapter.c     2015-03-03 19:46:44.064075811 -0500
@@ -186,6 +186,11 @@
        return (char*) &PersistentRam;
 }
 
+char* get_undo_memory()
+{
+       return (char*) &UndoState;
+}
+
 int get_memory_size()
 {
        return sizeof(PersistentRam);
diff -ur wp31s_r3756/QtGui/QtEmulatorAdapter.h wp31s_fix_undo_r3756_20150303/QtGui/QtEmulatorAdapter.h
--- wp31s_r3756/QtGui/QtEmulatorAdapter.h       2014-11-15 13:19:35.000000000 -0500
+++ wp31s_fix_undo_r3756_20150303/QtGui/QtEmulatorAdapter.h     2015-03-03 19:46:40.076075907 -0500
@@ -23,6 +23,7 @@
 extern void forward_keycode(int);
 extern void forward_key_released();
 extern char* get_memory();
+extern char* get_undo_memory();
 extern int get_memory_size();
 extern char* get_backup();
 extern int get_backup_size();
diff -ur wp31s_r3756/QtGui/QtEmulator.cpp wp31s_fix_undo_r3756_20150303/QtGui/QtEmulator.cpp
--- wp31s_r3756/QtGui/QtEmulator.cpp    2014-11-15 13:19:35.000000000 -0500
+++ wp31s_fix_undo_r3756_20150303/QtGui/QtEmulator.cpp  2015-03-03 19:46:47.568075727 -0500
@@ -774,15 +774,19 @@
        }
 
        int memorySize=get_memory_size();
-       if(memoryFile.size()!=memorySize)
+       if(memoryFile.size()!=memorySize && memoryFile.size()!=2*memorySize)
        {
-               memoryWarning(memoryFile.fileName()+" expected size is "+QString::number(memorySize)
+               memoryWarning(memoryFile.fileName()+" expected size is "+QString::number(2*memorySize)
                +" but file size is "+QString::number(memoryFile.size()));
                return;
        }
 
        QDataStream dataStream(&memoryFile);
        int reallyRead=dataStream.readRawData(get_memory(), memorySize);
+       if(reallyRead==memorySize && memoryFile.size()==2*memorySize)
+       {
+               reallyRead=dataStream.readRawData(get_undo_memory(), memorySize);
+       }
        if(reallyRead!=memorySize)
        {
                memoryWarning("Error whilst reading "+memoryFile.fileName());
@@ -834,6 +838,10 @@
        QDataStream dataStream(&memoryFile);
        int memorySize=get_memory_size();
        int reallyWritten=dataStream.writeRawData(get_memory(), memorySize);
+       if (reallyWritten==memorySize)
+       {
+               reallyWritten=dataStream.writeRawData(get_undo_memory(), memorySize);
+       }
        if(reallyWritten!=memorySize)
        {
                memoryWarning("Cannot write "+memoryFile.fileName());
diff -ur wp31s_r3756/storage.c wp31s_fix_undo_r3756_20150303/storage.c
--- wp31s_r3756/storage.c       2015-02-07 13:09:33.000000000 -0500
+++ wp31s_fix_undo_r3756_20150303/storage.c     2015-03-03 19:46:33.512076065 -0500
@@ -415,6 +415,7 @@
        init_state();
        checksum_all();
        fwrite( &PersistentRam, sizeof( PersistentRam ), 1, f );
+       fwrite( &UndoState, sizeof( UndoState ), 1, f );
        fclose( f );
 #ifdef DEBUG
        printf( "sizeof struct _state = %d\n", (int)sizeof( struct _state ) );
@@ -436,6 +437,7 @@
        FILE *f = fopen( STATE_FILE, "rb" );
        if ( f != NULL ) {
                fread( &PersistentRam, sizeof( PersistentRam ), 1, f );
+               fread( &UndoState, sizeof( UndoState ), 1, f );
                fclose( f );
        }
        f = fopen( BACKUP_FILE, "rb" );