TSCBL Cassette Loader - Code Review

[baktra]

Hello assembler coders,

 

I would like to ask you to review code for the TSCBL cassette loader. it is nothing else than binary loader for the "C:" device.

If you can spot any fatal mistakes...

 

https://github.com/baktragh/turgen_tape_loaders/tree/master/standard_tscbl

 

This loader should be the ultimate result of my effort to provide an open source replacement for the "exclamation mark" loader.

Thanks a lot.

 

 

[Rybags]

It's a long time and I think I lost the code but I wrote a tape binary loader in the mid 80s.

If using the C device rather than SIO there's potential if your code isn't efficient that a lot of short segments could cause overrun before the next block can be read.

[phaeron]

          jmp COLDSV              ;Otherwise perform cold start

Recommend killing NMIs before jumping to COLDSV, I have seen rare crashes due to unfortunate NMIs being triggered early in the cold start code.

 

There's also a bunch of opportunities to tighten up the code, if it would benefit from knocking off a sector: tail call to CIOV, use the returned N flag instead of re-testing status, more concise 16-bit address calc routines, etc.

 

[xxl]

          lda #255                ;Check for 255 255
          cmp BLSEGHEAD
          bne GS_ENDA
          cmp BLSEGHEAD+1
          bne GS_ENDA             ;If 255 255 not found, continue
          
          sta BL_HDR_FOUND        ;Indicate header was found

---

lda BLSEGHEAD

and BLSEGHEAD+1

cmp #$ff

bne GS_ENDA

 

?

[dmsc]

Hi!

23 hours ago, baktra said:

Hello assembler coders,

 

I would like to ask you to review code for the TSCBL cassette loader. it is nothing else than binary loader for the "C:" device.

If you can spot any fatal mistakes...

 

https://github.com/baktragh/turgen_tape_loaders/tree/master/standard_tscbl

 

This loader should be the ultimate result of my effort to provide an open source replacement for the "exclamation mark" loader.

Thanks a lot.

Only a fast look, with some changes it will be about 24 bytes shorter, probably more. If you are interested, I could send you a pull-request tomorrow.

 

Have Fun!

[ivop]

4 hours ago, xxl said:

          lda #255                ;Check for 255 255
          cmp BLSEGHEAD
          bne GS_ENDA
          cmp BLSEGHEAD+1
          bne GS_ENDA             ;If 255 255 not found, continue
          
          sta BL_HDR_FOUND        ;Indicate header was found

---

lda BLSEGHEAD

and BLSEGHEAD+1

cmp #$ff

bne GS_ENDA

 

?

 

deleted, because I mistakenly read ora instead of and....

Edited August 10, 2019 by ivop

[baktra]

I would like to thank everyone for suggestions. Much appreciated as I am not one of the best assembler 6502 coders. I used to write a lot of code in assembler (not 6502), but I coded for correctness, readability and maintainability (so when someone else reads the code, the number of WTFs per minute is low)

 

I was mainly concerned with correctness too, but it is the nature of the assembler coders to shorten to code. This is good, though I am afraid it will not shorten the code by one cassette block. But more space for fixes, longer program title or anything else is pretty useful too.

 

xxl

thanks for the suggestion

 

dmsc

I would appreciate the pull request

 

rybags

Yes, too many small segments could hurt. With this loader, I will stick to the "C:" device. I can compensate with slightly longer IRGs

generated by Turgen or XEX2CAS after blocks with too many segments. Anyway, these two also elongate IRGs after block with INIT vectors already. Having a SIOV-based binary loader would be good addition to the fleet. Especially if it is using blocks longer than 128 bytes and doesn't reside under ROM OS.

 

phaeron

Killing the NMIs will be included. Thanks for the suggestion. Then let us see what dmsc tightens up.

[dmsc]

Hi!

7 hours ago, baktra said:

I would like to thank everyone for suggestions. Much appreciated as I am not one of the best assembler 6502 coders. I used to write a lot of code in assembler (not 6502), but I coded for correctness, readability and maintainability (so when someone else reads the code, the number of WTFs per minute is low)

 

I was mainly concerned with correctness too, but it is the nature of the assembler coders to shorten to code. This is good, though I am afraid it will not shorten the code by one cassette block. But more space for fixes, longer program title or anything else is pretty useful too.

 

xxl

thanks for the suggestion

 

dmsc

I would appreciate the pull request

 

Done, see https://github.com/baktragh/turgen_tape_loaders/pull/1 

 

I included  @xxl's and @phaeron's suggestions and finally managed to remove 96 bytes from the bootloader, now it is only 387 bytes. To reduce it further, you can move the "CONFIG_*" variables from the current position at the end of the boot-loader to the code (replace "LDA CONFIG_FD" with "LDA #CONFIG_FD_VALUE", this will be 8 bytes less.

 

Have Fun!

[baktra]

Thanks for all suggestions. I will review and incorporate. I cannot promise incorporating all, especially inlininig. You might have noticed, that the loader's core is separated from not so essential parts,  so that the non-essential parts could be destroyed by the binary load file and the essential parts are close to address 2048.

[dmsc]

Hi!

8 hours ago, baktra said:

Thanks for all suggestions. I will review and incorporate. I cannot promise incorporating all, especially inlininig. You might have noticed, that the loader's core is separated from not so essential parts,  so that the non-essential parts could be destroyed by the binary load file and the essential parts are close to address 2048.

Ok, I moved things around and now the "main" loader part - up to the "JMP (INITAD)" - ends at $8E7, instead of your original that ends at $8F3, so it needs 12 bytes less.

 

Have Fun!

[baktra]

I have merged the code, so all of us can review it again.

Thanks a lot!