Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RS232 use affects disk access #357

Closed
AussieSmitty opened this issue Aug 15, 2021 · 45 comments
Closed

RS232 use affects disk access #357

AussieSmitty opened this issue Aug 15, 2021 · 45 comments

Comments

@AussieSmitty
Copy link

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
Copy link
Collaborator

Whammo commented Aug 15, 2021

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
Copy link
Author

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
Copy link
Collaborator

Whammo commented Aug 16, 2021

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
Copy link
Author

AussieSmitty commented Aug 16, 2021

Hi Whammo,

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

Steve

@Whammo
Copy link
Collaborator

Whammo commented Aug 16, 2021

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

@AussieSmitty
Copy link
Author

AussieSmitty commented Aug 16, 2021

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
Copy link
Owner

jkotlinski commented Aug 16, 2021

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:
#95 (comment)

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

@jkotlinski
Copy link
Owner

jkotlinski commented Aug 16, 2021

I think one clue to the problem can be found here:

lda $ba ;last used device number

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
Copy link
Collaborator

Whammo commented Aug 17, 2021

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

@jkotlinski
Copy link
Owner

jkotlinski commented Aug 17, 2021 via email

@Whammo
Copy link
Collaborator

Whammo commented Aug 17, 2021

SETUP 8 DEVICE LS

??

@AussieSmitty
Copy link
Author

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
Copy link
Owner

jkotlinski commented Aug 17, 2021 via email

@Whammo
Copy link
Collaborator

Whammo commented Aug 17, 2021

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
Copy link
Collaborator

Whammo commented Aug 17, 2021

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
Copy link
Owner

jkotlinski commented Aug 17, 2021

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
Copy link
Collaborator

Whammo commented Aug 17, 2021

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
Copy link
Collaborator

Whammo commented Aug 18, 2021

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
Copy link
Collaborator

Whammo commented Aug 18, 2021

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
Copy link
Owner

jkotlinski commented Aug 18, 2021 via email

@Whammo
Copy link
Collaborator

Whammo commented Aug 18, 2021

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
Copy link
Collaborator

Whammo commented Aug 18, 2021

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
Copy link
Collaborator

Whammo commented Aug 18, 2021

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
Copy link
Collaborator

Whammo commented Aug 18, 2021

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
Copy link
Contributor

polluks commented Aug 19, 2021

SETLFS, and SETNAM

Kernal routines help porting to C128...

@Whammo
Copy link
Collaborator

Whammo commented Aug 23, 2021

+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
Copy link
Owner

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
Copy link
Collaborator

Whammo commented Aug 25, 2021

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
Copy link
Owner

jkotlinski commented Aug 25, 2021 via email

@Whammo
Copy link
Collaborator

Whammo commented Aug 25, 2021

Any problem with vectoring Kernal LOAD & SAVE?

@jkotlinski
Copy link
Owner

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
Copy link
Collaborator

Whammo commented Aug 25, 2021

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
Copy link
Collaborator

Whammo commented Aug 25, 2021

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

@jkotlinski
Copy link
Owner

jkotlinski commented Aug 25, 2021

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

  • - file scratch in v.fs should be rewritten to check for errors. maybe that code can use SEND-CMD instead of OPEN/CLOSE, or at least OPEN should be followed by IOABORT.
  • - the device# fixup in disk.asm, where device# is "transformed" to 8 and up, is complete nonsense and should be removed.
  • - 10 device s" foo" $7000 loadb <-- this seems to get stuck reading the error channel! (in _errorchread) I'm not sure, but reading the error channel from devices that do not exist is maybe a bad idea? I am thinking it would be good to rewrite LOADB/SAVEB in Forth, using the nice primitives in io.fs and open.fs , and doing all error handling with IOABORT.

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

@Whammo
Copy link
Collaborator

Whammo commented Aug 25, 2021

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

Or rather shares _errorchread

@Whammo
Copy link
Collaborator

Whammo commented Aug 26, 2021

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

@jkotlinski
Copy link
Owner

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
Copy link
Owner

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

@Whammo
Copy link
Collaborator

Whammo commented Aug 27, 2021

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
Copy link
Collaborator

Whammo commented Aug 27, 2021

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

@Whammo
Copy link
Collaborator

Whammo commented Aug 27, 2021

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
Copy link
Owner

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
Copy link
Collaborator

Whammo commented Aug 28, 2021

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

@Whammo
Copy link
Collaborator

Whammo commented Aug 28, 2021

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants