Because I believe in collaboration: https://github.com/kervinck/gigatron-rom/pull/248
This is a combined pull-request that could be split as needed.
- The first commit upgrades GLCC to the latest version
- The next seven commits (up to 78f3021) are a ROMv6 release. The native code itself is essentially dev rom, that is, an approximation of what ROMv5 could have been (bugs fixed plus good support for expansion cards). The application selection removes pictures and tetronis, adds at67
s invaders (pristine gt1) and hans61's gtmine. New credits application gives credits for all these applications. At this point the dev rom is the same as ROMv6 save for romTypeValue.
- The last tree commits upgrade the dev rom to dev7, without changing the application selection other than a slightly faster Mandelbrot (found another 25%) and having a version of MSCP in Apps (but not included in the ROM)
All three items can be discussed independently to best satisfy the wishes of our community.
I'll update the pull request accordingly.
The pull request does not provide the dev rom binaries but they can be found in my master branch.
A pull request (to be discussed)
Forum rules
Be nice. No drama.
Be nice. No drama.
Re: A pull request (to be discussed)
Pull request won't be accepted as per https://forum.gigatron.io/viewtopic.php?t=19, I don't have the hours required to review the amount of changes you have made to the root/core.
The simple solution is to refactor it all so that it sits in your Contrib area, as everyone else, (including myself has done), see the above forum thread.
The simple solution is to refactor it all so that it sits in your Contrib area, as everyone else, (including myself has done), see the above forum thread.
Re: A pull request (to be discussed)
Hello at67.
Three points and a proposal:
My gut feeling is that we should try to agree. The purpose of this pull request is precisely to give my code to the community without strings attached.
I propose to reshuffle the commit as follows:
Three points and a proposal:
- GLCC update: I see no problems maintaining a version of glcc inside my Contrib dir. But this means that one should also revert the commit that creates the subdirectory Compilers, which currently contains only glcc (outdated). I will prepare a new pull request with only this change. We can agree that this is non-controversial.
- ROMv6: This is in fact a small change because the native code is from the current dev rom. The application changes are mostly trivial and can be revisited. The hardest part is janitorial: making sure everything is versioned correctly, that interface.json is updated accordingly, etc. This makes little sense in a Contrib directory.
- Dev7: This is a substantial change of native code. It is well tested at this point and known to be very backward compatible. This is why I thought it suitable for a dev rom because, well, a dev rom is tentative, not definitive. But I know this is a big one. It would not be hard to fold this into a Contrib directory, partly because this was intended as an experiment rather than a release.
My gut feeling is that we should try to agree. The purpose of this pull request is precisely to give my code to the community without strings attached.
I propose to reshuffle the commit as follows:
- Move GLCC under Contrib/lb3361 and update it there.
- Keep the ROMv6 changes into the main directories, because (a) it is a small change, (b) it is a fair approximation of what we know Marcel had in mind, and (c) because there is value in providing a stable development target to app developers, with properly versionned syscalls and interface.json. This would improve the life expectancy of the official gigatron rom, which I see as a benefit our community
- Move all Dev7 developments under Contrib/lb3361.
Re: A pull request (to be discussed)
The Compilers folder was mutually agreed to be beneficial to multiple users using both compilers and to have minimal impact on software compatibility. There's no need to revert this change.
This is how all pull requests to core should be, small, thoroughly tested and community approved. But any changes to core *have* to be 100% backward compatible with all previous ROM applications, this was one of the features that Marcel and I discussed in the early days and one of the things he charged me with when he gave me access to the main repo. This means core changes need to be thoroughly tested and reviewed for backwards compatibility and regressions.
Contrib devroms are temporary and for testing, they never belong in core, they belong in each user's Contrib folder as you eventually worked out.
Do whatever you like in your own Contrib folder, (as stated originally by Marcel both in the forums and in the documentation).
Re: A pull request (to be discussed)
Then it seems logical to update GLCC in place ?
I fully agree with this! This especially important when we are talking about a named rom instead of a devrom. In Marcel's guidelines, a named ROM should only contain permanently versioned apps (that will never change) and properly versioned syscall names published in interface.json. It is easy to forget something. Even Marcel forgot a ROM type check in the v5a cardboot. In fact, thoroughly testing this step was the *only* reason for creating a staged ROMv6 version on my GitHub. After a couple months, I believe I have it correct. Yet it is still necessary that other people also believe it to be correct.at67 wrote: ↑05 Jul 2023, 23:19This is how all pull requests to core should be, small, thoroughly tested and community approved. But any changes to core *have* to be 100% backward compatible with all previous ROM applications, this was one of the features that Marcel and I discussed in the early days and one of the things he charged me with when he gave me access to the main repo. This means core changes need to be thoroughly tested and reviewed for backwards compatibility and regressions.
Well, Contrib devroms belong to the Contrib directories because they are Contrib devroms, no need to work this out.at67 wrote: ↑05 Jul 2023, 23:19Contrib devroms are temporary and for testing, they never belong in core, they belong in each user's Contrib folder as you eventually worked out. Do whatever you like in your own Contrib folder, (as stated originally by Marcel both in the forums and in the documentation).
I often prefer to do such work in my own repositories because things can change quickly during the development process. You do not want ten Contrib pull requests per day! At some point things get less likely to change. Maybe that's a good time for moving into Contrib? I have a lot of contributions like this, ranging from hardware expansion boards to software for reading SD cards on the SPI port. These things are pretty well tested in fact. Even dev7 is pretty well tested by now and highly compatible. The problem with dev7 is not that it is not well tested, it is that it represents a substantial departure that might not be desirable in the context of Marcel's repository.
-
- Posts: 68
- Joined: 16 Jul 2019, 09:19
- Location: UK
Re: A pull request (to be discussed)
I'm keen to see further ROMs in the "mainline" series. Naturally it's important that additions are well tested, documented, and agreed to be useful by the community (although what the process for that looks like, I don't know), and backwards and forwards compatibility is essential.
If it would be useful, I'd like to volunteer to write some automated functional tests for the mainline series ROMs. I can't promise a timescale, but it doesn't seem too hard to check that most of the vCPU instructions and SYS Functions work correctly (including meeting their timing requirements).
If it would be useful, I'd like to volunteer to write some automated functional tests for the mainline series ROMs. I can't promise a timescale, but it doesn't seem too hard to check that most of the vCPU instructions and SYS Functions work correctly (including meeting their timing requirements).
Re: A pull request (to be discussed)
I have now reduced this pull request to only the GLCC update, a big commit strictly confined to Compilers/glcc.
More to come for ROMv6 later, in small commits.
The proposed ROMv6 (not a release, just a proposal) is visible in my dev repo: https://github.com/lb3361/gigatron-rom (master branch). Useful tests include (a) functional tests (how well does the rom work from the menu), (b) compliance tests (whether interface.json is accurate and whether all the programs have version suffixes, and (c) compatibility tests (how well the rom runs past gt1 files). Note that the native code in this rom is 99.99% similar to the current dev rom. The vCPU implementation has not changed since v5a. Most sys calls are the same as v5a. The changes are new sys calls. The only compatibility issue I know is related to vIRQ handling in v6502 mode, something that is only exercised in the Apple-1 emulator: the oldest versions can occasionally misbehave, the latest version contains a patch to prevent this.qwertyface wrote: ↑06 Jul 2023, 15:16 I'm keen to see further ROMs in the "mainline" series. Naturally it's important that additions are well tested, documented, and agreed to be useful by the community (although what the process for that looks like, I don't know), and backwards and forwards compatibility is essential. If it would be useful, I'd like to volunteer to write some automated functional tests for the mainline series ROMs. I can't promise a timescale, but it doesn't seem too hard to check that most of the vCPU instructions and SYS Functions work correctly (including meeting their timing requirements).
Re: A pull request (to be discussed)
Here is a tentative pull-request for a ROMv6, to be scrutinized as needed.
Details of each commit discussed in https://github.com/kervinck/gigatron-rom/pull/249
Two commits of this pull request touch code created by at67 such as adding Invader to the app selection. This is a path-breaking program that, in the context of a mainline rom, showcases both the Gigatron capabilities and at67s contributions.
Details of each commit discussed in https://github.com/kervinck/gigatron-rom/pull/249
Two commits of this pull request touch code created by at67 such as adding Invader to the app selection. This is a path-breaking program that, in the context of a mainline rom, showcases both the Gigatron capabilities and at67s contributions.
Re: A pull request (to be discussed)
At67 does not wish to see Invader or any of his applications in ROMv6 (see his own words in the pull request; let's abstain from discussing his decision in this thread). This leaves about 12KB of space for a different application. Possible options:
- Restore the Pictures application as in ROMv5a.
- Add a different game (see https://forum.gigatron.io/viewtopic.php?p=1647#p1647 for ideas)
Last edited by lb3361 on 12 Jul 2023, 09:58, edited 1 time in total.