jkotlinski / durexforth

Modern C64 Forth
Other
230 stars 28 forks source link

RS232 use affects disk access #357

Closed AussieSmitty closed 3 years ago

AussieSmitty commented 3 years ago

There appears to be disk I/O issues after utilising RS232. Here is the code I use to set the RS232 port for 1200 baud.

( RS232 on C64 )

: setup ( -- ) 14 53280 c! \ light blue border 154 emit \ text colour 6 53281 c! \ background dark blue [ $fb stx, \ store a copy of xreg 8 lda,# \ low byte of params $fd sta, \ store at $fd 0 lda,# \ hi byte of params $fe sta, \ store at $fe $2 lda,# \ length of filename $fd ldx,# \ lo byte params add $0 ldy,# \ hi byte params add $ffbd jsr, \ call kernal setnam 2 lda,# \ 2 = RS232 device# tax, \ set device=2 0 ldy,# \ no secondary # $ffba jsr, \ setlfs $ffc0 jsr, \ open $fb ldx, ] \ restore xreg 147 emit ; \ clear screen

Here is the routine I use to send a byte (which is working fine)

: sendbyte ( byte -- ) $fc c! [ $fb stx, \ save xreg 2 ldx,# \ rs232 device # $ffc9 jsr, \ chkout $fc lda, \ byte to send $ffd2 jsr, \ chrout 3 ldx,# \ screen device # $ffc9 jsr, \ chkout $fb ldx, ] ; \ restore xreg

I discovered after compiling (F7) the code above, and using it, e.g.:

setup 13 sendbyte

then go back to the editor (using 'v'), any change I then make to the code can't be saved.

Perhaps the issue lies in the way I am setting or using the RS232, but at this stage it appears to be that DurexForth's access to disk kernal is somehow affected by the use of RS232.

Thanks in advance for any help. Save not working

Whammo commented 3 years ago

May I suggest, you're not trapping for i/o errors i.e. checking for carry after kernal calls. Things may be in an 'undefined' state. If you would, try using the IO words and see if disk access is impared.

AussieSmitty commented 3 years ago

Hi Whammo,

Thanks for offering to assist.

I'm not sure what you're suggesting. Are you saying I need to call something first to check the status before I try to save to a disk?

Editing existing Forth code should allow a save, but as the image I captured shows, I'm not getting any disk activity (normally shown with a green light in the bottom right rectangle). Saving works until I use RS232, but not sure how to 'trap for i/o errors'. Thanks for further guidance on how to resolve this.

Regards, Steve

Whammo commented 3 years ago

Device 2 is both input and output.

include io
: sendbyte ( byte -- ) $fc c! [
$fb stx, \ save xreg
2 ldx,# \ rs232 device #
$ffc9 jsr, \ chkout
$fc lda, \ byte to send
$ffd2 jsr, \ chrout
3 ldx,# \ screen device #
$ffc9 jsr, \ chkout        <------     2 is still input
$fb ldx, ] \ restore xreg 
clrchn ;  \   <----- clrchn
AussieSmitty commented 3 years ago

Hi Whammo,

I think you've seen my error, thanks. I'll try it tonight and confirm your genius status!

Steve

Whammo commented 3 years ago

I've been doing this for nearly forty years, It doesn't take a genius to understand it's not genius. 😉

AussieSmitty commented 3 years ago

Hi Whammo,

Still not working for me I'm afraid :(

I looked back at my working assembly code and I do the same as my Forth code. I did notice that I also called CLRCHN just to make sure the IO is clear. But even if I introduce this to my 'sendbyte' word, saving from the v editor again fails.

: sendbyte ( byte -- ) $fc c! [ $fb stx, \ save xreg 2 ldx,# \ rs232 device # $ffc9 jsr, \ chkout $fc lda, \ byte to send $ffd2 jsr, \ chrout 3 ldx,# \ screen device # $ffc9 jsr, \ chkout $ffcc jsr, \ clrchn 0 ldx, \ revert keyboard as input $ffc9 jsr, \ chkout $ffcc jsr, \ clrchn $fb ldx, ] ; \ restore xreg

I feel like you are onto something, but I still can't get my head around where the issue lies. Perhaps I need to close the RS232 everytime I've sent a byte, but that I'm sure isn't supposed to be needed. I'm opening RS232 with a logical file number of 2 as well as device 2. Which means the v editor should still be able to open the disk channel for saving or reading as it would use a different logical file number.

I even tried using a different logical file number for rs232, but this made no difference.

Let me know if there is anything I can do to help as I'm keen to get this working for both RS232 and disk operations within my code.

Regards, Steve

jkotlinski commented 3 years ago

I see an OPEN, but is the CLOSE missing, before you try to save to disk?

This advice was helpful to me, on how to order operations: https://github.com/jkotlinski/durexforth/issues/95#issuecomment-748296717

EDIT: I tried adding a CLOSE and it didn't seem to help. So probably it is something else.

jkotlinski commented 3 years ago

I think one clue to the problem can be found here: https://github.com/jkotlinski/durexforth/blob/5c80c6e09639ccd76f3eb86ec0c19f0123a29cd8/disk.asm#L154 If I change this line to "lda #0", I can get things to work again.

Right now it's a bit late, so I can't tell immediately why things go wrong, or what would be a good fix. But I'm open for suggestions.

Whammo commented 3 years ago

Smitty, Is any disk access- for instance LS- disallowed, or is it only writing from V? Also, does run/stop restore fix it?

jkotlinski commented 3 years ago

It seems like SETUP LS wont work either.

On Tue, 17 Aug 2021 at 02:06, Whammo @.***> wrote:

Smitty, Is any disk access- for instance LS- disallowed, or is it only writing from V?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jkotlinski/durexforth/issues/357#issuecomment-899896195, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAY34OY2IJAMV3HOO53Z5ZLT5GRYVANCNFSM5CGDZPJA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

Whammo commented 3 years ago

SETUP 8 DEVICE LS

??

AussieSmitty commented 3 years ago

Hi Whammo & jkotlinski,

You've nailed it! :) If I do an 8 device, disk io for the directory or to save from the v editor now all works.

Thank you, now to my robotic projects with my C64 hardware and RS232 connections to external hardware.

Regards, Steve

jkotlinski commented 3 years ago

Great! I guess theres still a problem to be solved, this setup seems error prone when using multiple devices.

On Tue, 17 Aug 2021 at 13:48, Steve Smit @.***> wrote:

Hi Whammo & jkotlinski,

You've nailed it! :) If I do an 8 device, disk io for the directory or to save from the v editor now all works.

Thank you, now to my robotic projects with my C64 hardware and RS232 connections to external hardware.

Regards, Steve

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jkotlinski/durexforth/issues/357#issuecomment-900227283, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAY34O6XZJQBI6TFQCOYE5TT5JD7FANCNFSM5CGDZPJA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

Whammo commented 3 years ago

The good part is that $ba is handled by the Kernal and should always be the current (valid) device. DEVICE should be it's own zeropage byte and let the Kernal handle $ba

disk_io_setnamsetlfs ;reused by both loadb and saveb
    jsr SETNAM
    lda $ba     ;last used device number  <---  ldx device
    and #3      ;Make 0-3 possible numbers     *snip*
    ora #8      ;Transform to 8-B                        *snip*
    tax                                                                     *snip*
    lda #1
    ldy #0      ;if load: 0 = load to new address, if save: 0 = dunno, but okay...
    jmp SETLFS  ;End with JMP instead of jsr/rts to save a jsr/rts pair...
Whammo commented 3 years ago

Using $02 for DEVICE and the current release a preliminary patch:

8 2 c!        \ default device number to device
2 154e c!   \ DEVICE sta $02
2 155b c!   \ disk_io_setnamsetlfs lda $02
2 29a7 c!   \ open.fs $02 ldx,

These are the references (I have found) where the Kernal expects device to be disk and will update $ba

jkotlinski commented 3 years ago

that's food for thought. I don't agree DEVICE should use a zeropage byte, zeropage is such a precious resource.

i'm wondering if it makes sense to let DEVICE be a plain VARIABLE, default value 8, which could then be used by OPEN, LOADB and SAVEB, or any word that would like to use it.

to change device: e.g. "9 device !"

main drawback as i see it is, it would be a breaking change and cause Durexforth 4.

the other option is to just change the behavior of "device", like you suggest. so it kind of works the same (except maybe calling device should not set $ba then)

Whammo commented 3 years ago

There seems to be a little more to it than is implied by my patch.

Setting $ba to 2, while device is 8 then executing LS returns 0,0, ok 0,0 but no directory. After that it works correctly.

Whammo commented 3 years ago

Hmmm.. If load_binary_laddr_hi = 0 then load_binary_status = fail then jsr _errorchread this catches a bad load address or 0 save filename length.

   lda $ba      ;last used device number
    and #3      ;Make 0-3 possible numbers
    ora #8      ;Transform to 8-B

I don't understand why this happens, but 2 works out to $a

Whammo commented 3 years ago

I also notice .disk_io_setnamsetlfs calls SETNAM before SETLFS.

References say, call the SETLFS, and SETNAM routines and I can't seem to find another code example of SETNAM before SETLFS anywhere. Admittedly, I haven't tried very hard.

jkotlinski commented 3 years ago

The file loading was based on this code:

https://codebase64.org/doku.php?id=base:loading_a_file

On Wed, 18 Aug 2021 at 11:51, Whammo @.***> wrote:

I also notice .disk_io_setnamsetlfs calls SETNAM before SETLFS.

References say, call the SETLFS, and SETNAM routines and I can't seem to find another code example of SETNAM before SETLFS anywhere. Admittedly, I haven't tried very hard.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jkotlinski/durexforth/issues/357#issuecomment-900978504, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAY34OZ4PIY5LPLHJNJQ7J3T5N7BVANCNFSM5CGDZPJA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

Whammo commented 3 years ago

There are people that can solve this at a glance. I am assuredly not one of them. It wouldn't be ruining my fun if somebody stepped up. greg-king5 ?

Johan, I'm grateful for your time and attention and proud to have had a finger in the pie. Meanwhile, I'll enjoy exploring the implications of my patch.

Whammo commented 3 years ago

BTW, SETLFS, and SETNAM are incredibly simple routines that don't share zero page, it was obvious when I looked at the Kernal source that it matters not what order they're called. Heck, they're so simple, it might be worth loosing the overhead and go directly from the data stack to zero page skipping the calls.

*****************************
FDF9 85 B7 STA $B7
FDFB 86 BB STX $BB
FDFD 84 BC STY $BC
FDFF 60 RTS
*****************************
FEOO 85 B8 STA $B8
FE02 86 BA STX $BA
FE04 84 B9 STY $B9
FE06 60 RTS
****************************
Whammo commented 3 years ago

So, DEVICE being irrelevant, INCLUDED shares _errorchread, knowing what we know now, we have the most excellent specifications below. Chuck Moore was correct about the power of 'code, then code again' with forth. This stuff practically writes itself.

LOADB ( filenameptr filenamelen dst -- endaddress ) load binary file
SAVEB (save binary file)
;  - 7000 71ae s" base" saveb #save file from 7000 to 71ae (= the byte AFTER the last byte in the file)
Whammo commented 3 years ago

But what I'm missing, without a doubt is that this is highly space optimized code, written by experts and that I am crunchy, and taste good with catsup. 😊

polluks commented 3 years ago

SETLFS, and SETNAM

Kernal routines help porting to C128...

Whammo commented 3 years ago
+BACKLINK "device", 6
    lda LSB,x
    sta $ba
    inx
    rts

to

+BACKLINK "device", 6
    lda LSB,x
    sta device
    inx
    rts
device = * 
    php  ; $08

Then point $ba reads and writes to device.

jkotlinski commented 3 years ago

that code would not work straight off: because "device" assembler label cannot be accessed from Forth code.

it could also be that what's in place is actually good enough...

Whammo commented 3 years ago

Just for it to be documented, the (non)issue is reading $ba to subsequently set $ba with SETLFS, and:

.disk_io_setnamsetlfs ;reused by both loadb and saveb
    jsr SETNAM
    lda $ba     ;last used device number
    and #3      ;Make 0-3 possible numbers
    ora #8      ;Transform to 8-B
    tax
    lda #1
    ldy #0      ;if load: 0 = load to new address, if save: 0 = dunno, but okay...
    jmp SETLFS  ;End with JMP instead of jsr/rts to save a jsr/rts pair...

.disk_io_setnamsetlfs transforming device $02 to $0a.

jkotlinski commented 3 years ago

I have an idea for improvement - thats not a breaking change or needs further documentation: LOADB and SAVEB could ABORT with error message if called with $ba outside valid range.

On Wed, 25 Aug 2021 at 08:02, Whammo @.***> wrote:

Just for it to be documented, the (non)issue is reading $ba to subsequently set $ba with SETLFS, and:

.disk_io_setnamsetlfs ;reused by both loadb and saveb jsr SETNAM lda $ba ;last used device number and #3 ;Make 0-3 possible numbers ora #8 ;Transform to 8-B tax lda #1 ldy #0 ;if load: 0 = load to new address, if save: 0 = dunno, but okay... jmp SETLFS ;End with JMP instead of jsr/rts to save a jsr/rts pair...

.disk_io_setnamsetlfs transforming device $02 to $0a.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jkotlinski/durexforth/issues/357#issuecomment-905207810, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAY34O7HXPMZNNSBT25HZA3T6SBPBANCNFSM5CGDZPJA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

Whammo commented 3 years ago

Any problem with vectoring Kernal LOAD & SAVE?

jkotlinski commented 3 years ago

I'm not sure what you mean "problem with vectoring Kernal LOAD & SAVE"?

For the idea with direct zero page access instead of calling SETLFS - the implementation and zero page addresses seem exactly the same on C128. So I do not see a problem writing to zeropage $b8-$ba instead of calling SETLFS.

Whammo commented 3 years ago

One solution is redirecting the LOAD and SAVE vectors at $330 and $332 to insert a $ba check. However it seems Kernal LOAD and SAVE are presumed to work with device 2.

Whammo commented 3 years ago

On further thought, perhaps the only issue is that V needs to have it's own device settings and checks.

jkotlinski commented 3 years ago

I think there are multiple things wrong here. A non-exhaustive list of things to fix:

I'm not sure if this is feasible but above is what I'm thinking now :)

Whammo commented 3 years ago

LOADB/SAVEB are pretty simple, but thoroughly entangled with INCLUDED which is not trivial.

Or rather shares _errorchread

Whammo commented 3 years ago

Looking at open.fs to separate, streamline and make available SETLFS and SETNAM in open.

jkotlinski commented 3 years ago

I'm investigating if _errorchread can be removed. The Kernal can print some error messages on its own accord, if $9d is set to $40.

jkotlinski commented 3 years ago

Another issue: IOABORT does not contain strings for all errors.

Whammo commented 3 years ago

I'm investigating if _errorchread can be removed. The Kernal can print some error messages on its own accord, if $9d is set to $40.

It would be nifty to use the Kernal Msg for the ok prompt! 💯

Whammo commented 3 years ago

There is some optimizations that could be done with OPEN, but separating out SETLFS and SETNAM is pointless if it breaks OPEN.

Whammo commented 3 years ago

This ignores OPEN, it's low-level up to LOADB and SAVEB

: setlfs ( file# sa -- )
$b8 c! $b9 c! ;

: setnam ( nameaddr namelen -- )
$b7 c! $bb ! ;

: errchk  sr c@ 1 and if 
ar c@ else
0  then ioabort ;

:  kopen (  --  )
$ffc0 sys errchk ;

:  load  ( sa dst --  )
xr ! ar c! $ffd5 sys errchk ;

: save ( start end zeropage -- )
dup ar c! swap xr ! !
$ffd8 sys errchk ;

( filenameptr filenamelen dst -
- endaddress )
: loadb
1 0 setlfs -rot setnam 0 
swap load 2drop $ae @ ; 

: saveb ( start end nameaddr namelen -- )
  1 0 setlfs setnam $c1 save 2drop ; 
jkotlinski commented 3 years ago

I'm investigating if _errorchread can be removed. The Kernal can print some error messages on its own accord, if $9d is set to $40.

It would be nifty to use the Kernal Msg for the ok prompt! 💯

:) After some exploration, I'm not convinced that Kernal Error Messages should be enabled by default. Errors can be a bit confusing - e.g. "I/O ERROR #5" is not a very helpful message. It is more something that's nice to occasionally enable, when debugging.

Whammo commented 3 years ago

It might be worth flipping basic in and out to access the error messages at $a19e. the table is vectored by number @ $a328

Whammo commented 3 years ago

Here is a demo of basic error messages. This works with 1-30

marker foo

: berr ( errornum -- )
55 1 c! 1-  \ flip in basic
1 lshift $a328 + @ \ 16bit offset table
begin dup c@ dup 128 and 0= while \ last byte, hi bit set
emit 1+ repeat 128 - emit \ emit last byte
drop 54 1 c! cr ; \ flip out basic

$a364 is the location of cr 'ok' cr but those are null-terminated

jkotlinski commented 3 years ago

Since the reported problem is more or less solved now, I'm closing this to create specific follow-up issues.