Skip to content

swtp6809 simulator being contributed to open-simh project#357

Closed
rfromafar wants to merge 0 commit intoopen-simh:masterfrom
rfromafar:master
Closed

swtp6809 simulator being contributed to open-simh project#357
rfromafar wants to merge 0 commit intoopen-simh:masterfrom
rfromafar:master

Conversation

@rfromafar
Copy link
Copy Markdown

Back in the winter of 2021, I wrote a simulator for the SWTPC 6809 computer. The simulator was based on the existing swtp6800 simulator written by William Beech. I have decided that I would like to contribute this to the open-simh project. I have cleaned up the code somewhat so that it can reviewed. My next steps would be to create a user document similar to the one which was created for the swtp6800 simulator. This simulator emulates the following system:

  • SWTPC MP-09 CPU board with Dynamic Address Translation for 20-bit physical addressing
  • MC6809 CPU emulation
  • single BOOTROM emulating various EPROM sizes (replaces the 4 2K EPROMs on the MP-09 CPU board)
  • MP-B3 motherboard with 8 SS-30 I/O slots each assigned 4 bytes of addressing
  • MP-1M memory which emulates 1MB of RAM
  • MP-S serial card
  • DC-4 floppy controller (some modifications from the original used by swtp6800 simulator)
    It runs Flex 9 DSK images very well. I am willing to provide time for debugging and resolving any issues with this simulator.
    I am new to GIT and the project, so, any advice you can provide is appreciated. THANK YOU!

@pkoning2
Copy link
Copy Markdown
Member

pkoning2 commented Feb 22, 2024

Thanks!
If I understand correctly, this is mostly the work of William Beech with additions and modifications you wrote. It looks like he has a BSD-style or MIT-style license on the files, so that would suggest there isn't an issue there. I'm wondering, though, if you have mentioned to him that you've made this submission. While apparently not required it seems like a good courtesy to do so.

Apart from that, I'd like to hear from other SG members to confirm we're ok with the license on these files.

One other detail is that the CMake control files should be updated for this new simulator. @bscottm can you help with that?

@rfromafar
Copy link
Copy Markdown
Author

rfromafar commented Feb 22, 2024 via email

@rfromafar
Copy link
Copy Markdown
Author

rfromafar commented Feb 22, 2024 via email

@rcornwell
Copy link
Copy Markdown
Member

Couple points.

  1. There should be a document file describing how to run this simulator.
  2. The commit messages do not follow the established convention of simulator: change

@rfromafar
Copy link
Copy Markdown
Author

rfromafar commented Feb 22, 2024 via email

@rfromafar
Copy link
Copy Markdown
Author

I see that the builds for XP and Win10 failed required checks. I am thinking/hoping this is due to the fact that I did not convert my source files to DOS format using UNIX2DOS prior to check-in.

I have a question about next steps in the process...

If I check-in changes, such as converting to DOS format, would they still be under this pull-request? Or, do I make the necessary changes and then make another pull request?

BTW, I also need to check-in the simulator user guide (DOC file) and a copy of the SWTPC S-BUGE monitor BOOTROM file which I will do this weekend.

Thank you!
--Richard

@pkoning2
Copy link
Copy Markdown
Member

I think that the MS tools are capable of handling Unix line endings. However, the convention in this project is to use DOS line endings.
For cleanups, addressing review comments, or adding in missing elements such as a documentation file, adding them to the existing PR is a good answer. Work that significantly deviates from or generalizes what your current PR proposes is best done in a separate PR.
For example: do you want to proceed with this PR that is just 6809? Or do what was discussed, which is to cover the family and have your 6809 work as one variant? For the former, this PR is the way to do that, including any cleanups that should be done for it. But if you want to stop the 6809-only submission and replace it by a general one, I would recommend closing this PR and submitting a new PR for that new larger work instead (which I suppose might take some time to create).

@pkoning2
Copy link
Copy Markdown
Member

pkoning2 commented Feb 23, 2024

@bscottm do you have any idea what went wrong with the Windows based builds? They are cmake ones, but I can't decipher the logs to see what the problem is.

@bscottm
Copy link
Copy Markdown
Contributor

bscottm commented Feb 23, 2024

@bscottm do you have any idea what went wrong with the Windows based builds? They are cmake ones, but I can't decipher the logs to see what the problem is.

See PR #360. It's not clear to me what changed recently in either the MS runtime or CMake argument parsing. It came down to excess quotes passed on the CMake command line. CMake should have seen a single argument, i.e., "Visual Studio 17 2022", but it was actually seeing ""Visual Studio 17 2022"" (the extra quotes.)

@pkoning2
Copy link
Copy Markdown
Member

@bscottm do you have any idea what went wrong with the Windows based builds? They are cmake ones, but I can't decipher the logs to see what the problem is.

See PR #360.

Awesome, thanks for the fast response.

@bscottm
Copy link
Copy Markdown
Contributor

bscottm commented Feb 23, 2024

@pkoning2: For this to build with CMake, need to update cmake/simgen/packaging.py adding this simulator to the swtp_family of simulators. Then run generate.py to update or create the respective CMakeLists.txt.

@pkoning2
Copy link
Copy Markdown
Member

So, @rfromafar if you could do that update @bscottm outlined, and also rebase to the latest main branch to bring in the hotfix in PR #360, that would be great because it should then make all the tests run properly.

@rfromafar
Copy link
Copy Markdown
Author

rfromafar commented Feb 23, 2024 via email

rfromafar added a commit to rfromafar/simh that referenced this pull request Feb 25, 2024
… (Pull Request)

- updates to code while writing the simulator usage guide
- added copyright notice to relevant source code modules
- used unix2dos to format source files for DOS line terminator conventions
- source code files from swtp6800 with some changes: swtp_defs.h, dc-4.c
- source code files from swtp6800 that are unchanged: mp-s.
@rfromafar
Copy link
Copy Markdown
Author

I believe I have rebased the code to pick up the changes made to the master. I have made some additional commits over the weekend. The user guide is uploaded to the doc folder. Copyright notices have been added to source files which contain significantly "new" code. Writing the user guide made me realize that some of the devices could benefit from additional modifiers (config settings), so, the result should be a more user-friendly simulator.

@rcornwell
Copy link
Copy Markdown
Member

The commit ID's are still not following the convention. This format is a requirement. This is also to help determine what a given commit effects. There is a large number of simulators and having to look through the actual commit can cause a problem. Also the makefile and other non-simulator changes should be in a separate commit. You commit messages should be of the following format:

SWTP6809: basic commit reason.

Any more lines detailing changes as you see fit.

When looking a a commit I should be able to tell what simulator it might effect without having to dig around in the commit file list.

@bscottm
Copy link
Copy Markdown
Contributor

bscottm commented Feb 25, 2024

@rfromafar: Instead of merging the master branch, which inserts a merge commit, open-simh generally prefers rebasing, i.e.,

$ git fetch origin master
$ git rebase origin/master

Alternatively, because it's so frequently done:

$ git pull --rebase origin master

Rebasing avoids the merge commit, but you end up having to force the next push, i.e., git push --force ....

@rfromafar
Copy link
Copy Markdown
Author

Thank you all for the helpful comments. At this point, I am not planning on making any other changes in the immediate term to the swtp6809 simulator code. I did the " git pull --rebase origin master", but, likely too late. Wondering if this is salvageable, or, if I should create a new clean branch and add my files there to keep things clean. I think the only common file which I would have touched intentionally is "makefile" to add the target for swtp6809mp-09.

There is a matter of 2 files which are "common" to the swtp6800 and swtp6809 simulators:

  • mp-s.c (serial io) is used by the swtp6809 simulator, but, it is just a copy of the file from swtp6800 tree and therefore I am tempted to just reference mp-s.c from the swtp6800 simulator in the makefile to avoid having the same file under the swtp6809 tree.
  • dc-4.c (floppy controller) is used by both simulators, however, the dc-4.c version in the swtp6809 simulator has many updates. It will take some time to merge the code into one file. The other option is to rename the file in the swtp6809 tree to dc-4_6809.c at least for now.

The initial check-in comments would be something like "swtp6809: initial commit for contribution of new simulator for the SWTPC 6809 MP-09 computer system". Anyway, my branch should now be up to date to evaluate. Thanks again!

@bscottm
Copy link
Copy Markdown
Contributor

bscottm commented Feb 25, 2024

Thank you all for the helpful comments. At this point, I am not planning on making any other changes in the immediate term to the swtp6809 simulator code. I did the " git pull --rebase origin master", but, likely too late. Wondering if this is salvageable, or, if I should create a new clean branch and add my files there to keep things clean.

@rfromafar: Everything is salvageable. You can squash the commits via a git rebase -i. Do a git log, looking for your first commit in this series. Then copy the first 8 digits of the SHA of the commit that precedes your first commit in this series. You'll end up in your favorite editor (or nano, if you don't have EDITOR set in your environment), at which point you leave the first line alone and change all subsequent lines to squash. You may have to resolve some conflicts along the way... and then you'll end up being asked to consolidate the commit message.

Going with a new clean branch is also perfectly viable if rebasing and squashing commits is terrifying.

@markpizz
Copy link
Copy Markdown
Contributor

Since the developer of the SWTP6800 simulator is still around and still messing with this and/or other simulators the general simh concept has historically been that one should work with that developer to get their input relating to fixes, additions prior to making such changes.

@pkoning2
Copy link
Copy Markdown
Member

Since the developer of the SWTP6800 simulator is still around and still messing with this and/or other simulators the general simh concept has historically been that one should work with that developer to get their input relating to fixes, additions prior to making such changes.

He has been giving that input on the mailing list.

@rfromafar rfromafar closed this Apr 4, 2024
rfromafar added a commit to rfromafar/simh that referenced this pull request Apr 4, 2024
# This is the 1st commit message:

Richard Lukes - 18 FEB 2024
This is my initial commit of my swtp6809 emulator which is a modified version of the swtp6800 emulator.
The following changes are still outstanding:
- various code clean up tasks and removal of temporary comments
- bootrom.c - reset() appears to be performing attach() funtion, I plan to clean up soon
- mp-b3.c - want to add #define for 4 bytes per slot versus 16 bytes per slot
- dc-4.c - trying to understand the changes I made from the original dc-4.c in the swtp6800 emulator
- I will do additional testing, however, for now this boots Flex 9.0 and appears to be working fine

# This is the commit message open-simh#2:

In preparation of presenting for contribution as a "pull request".
General clean up of code.
Removed unneeded test prints that were used for debugging.
Removed swtp_sbuge_bin.h which implemented BOOTROM code internally. Users should use "ATTACH BOOTROM <sbuge.bin filename>".
Added reset() routine to CPU.  If last line in INI file is "RESET CPU" then simulator goes straight into BOOTROM from reset vector at $FFFE.
Tested with several FLEX 9 DSK files.
Known issues are:
1) Backspace (BS) does not seem to work when running Flex 9.
2) When simulator starts up the PC has a value of $FFFF (and not $FFFE).
3) No DMAF1/DMAF2 disk emulation which is required for support of UniFlex.
4) No documentation has been written yet. However, I am more than willing to put something together.
Note this code was developed using Debian on Raspberry Pi 4. There may be Unix/DOS file conversion issues.

# This is the commit message open-simh#3:

swtp6809 simulator being contributed to open-simh project open-simh#357 (Pull Request)
- updates to code while writing the simulator usage guide
- added copyright notice to relevant source code modules
- used unix2dos to format source files for DOS line terminator conventions
- source code files from swtp6800 with some changes: swtp_defs.h, dc-4.c
- source code files from swtp6800 that are unchanged: mp-s.

# This is the commit message open-simh#4:

CMake: Reduce excess quoting

cmake/cmake-builder.ps1 added quotes to arguments that contained spaces,
so that arguments printed correctly for progress output. This introduced
excess quotes that caused CMake (and likely other MS apps) confusion or
argument misinterpretation.

Instead of CMake seeing a single "Visual Studio 17 2022", CMake was
actually seeing "\"Visual Studio 17 2022\"".

This patch only adds the additional quotes when reporting progress or
emitting debug output. Otherwise, command line arguments are passed
unmolested.

# This is the commit message open-simh#5:

The user doc file for the swtp6809 simulator

SWTP 6809 Simulator Usage guide.
rfromafar added a commit to rfromafar/simh that referenced this pull request Apr 4, 2024
rfromafar added a commit to rfromafar/simh that referenced this pull request Apr 4, 2024
@bscottm
Copy link
Copy Markdown
Contributor

bscottm commented Apr 5, 2024

@pkoning2, @rcornwell, @rfromafar Umm... I was going to look into the cmake problem, but this appears OBE'd.

Actually, if the simulator has been added to the makefile, python3 cmake/generate.py should automagically take care of the CMake build setup.

(OBE: Overrun/Overwhelmed By Events.)

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 this pull request may close these issues.

5 participants