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

Failure to persist user/drive state #13

Closed
skx opened this issue Jun 20, 2024 · 3 comments · Fixed by #14
Closed

Failure to persist user/drive state #13

skx opened this issue Jun 20, 2024 · 3 comments · Fixed by #14

Comments

@skx
Copy link
Contributor

skx commented Jun 20, 2024

[I have a patch ready for this, which I will submit shortly]

When I was writing my emulator I wrote some test programs to cement my understanding of some of the expected behaviour. One simple script I wrote was ret.z80, which is a trivial binary which just tries to exit in a couple of different ways:

  • JP 0x0000
  • RST 0
  • RET
  • LD C, 0x00; CALL 0x0005

There is a binary which is located within that same directory which can be used for testing - but note that you really want to map this to a non-default drive. In my case I see this when I test:

$ ./target/debug/iz-cpm --disk-a /home/skx/Repos/github.com/skx/cpm-dist/A/ --disk-b  /home/skx/Repos/github.com/skx/cpmulator/samples/ 
iz-cpm https://github.com/ivanizag/iz-cpm
CP/M 2.2 Emulation
Press ctrl-c ctrl-c Y to return to host

A>b:
B>dir ret.*
RET     .COM  |  RET     .Z80
B>ret 1

B>ret 2

B>ret 3

B>ret 4

A>

Here you see that the execution of the binary succeeded in each of the invokations, each time the binary was launched and then terminated. However The last run switched back to the default A-drive.

You correctly pass this value along when reseting via the boot:

                      let user_drive = machine.peek(CCP_USER_DRIVE_ADDRESS);
                    cpu.registers().set8(Reg8::C, user_drive);

However that value is never set. I've made a couple of other changes to remove the "= 0" from the drive/user setting in the rest/state, and added the missing stores.

That allows things to work as expected.

@ivanizag
Copy link
Owner

Wow. I just wrote a comment on the other one :D

@ivanizag
Copy link
Owner

Great if you have a patch. Thanks

@skx
Copy link
Contributor Author

skx commented Jun 20, 2024

No burden, I have an hour just now and I'll write up the things that jumped out at me.

Probably no patches for the others though, at least not tonight! (And I appreciate some things might be a matter of interpretation so I'm happy for you to close any report I make as "Won't fix :)" )

ivanizag added a commit that referenced this issue Jul 9, 2024
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

Successfully merging a pull request may close this issue.

2 participants