Closed fayep closed 1 year ago
Thanks!
Are the changes in src/vdp/vdp-console8, src/vdp/vdp-quark103 etc necessary for this to build on mac? That code comes from the Agon VDP firmware, so I like to keep the diff there minimal to make updating firmware versions easy. If it's necessary then I could try to get those changes merged into the official firmware.
I've added the changes to build.rs. Let me know if that's insufficient to get the build working.
FYI I had spotted the bug you found with the day of year value. @breakintoprogram fixed this a little while back. He changed the RTC protocol to pack the data better/tighter, and has also made the corresponding changes to MOS.
Many/most of the changes here are explicit type conversions which are effectively present in later versions of the VDP code too, just in a slightly different form. I went on a “fix all the compiler warnings” tear a while back.
Historically, the VDP code had all of these issues in it because the Arduino IDE by default compiles with quite lax settings. It would silently let data type coercion on larger integers into smaller values happen. In this case, rather than changing the source to fix (which of course is usually the right thing to do), these issues are probably better resolved by changing compiler settings.
An important thing to consider/remember here is that the VDP code in the emulator is a snapshot of the VDP codebase at a point in time with only minimal changes to allow it to work in the emulator. Bug fixes to code that changes functionality, like the date protocol changes here, will make the emulator incompatible with code running on Agon hardware.
Bug fixes like that are of course welcome tho, but they should be raised against the agon-vdp repository
Thanks both. I see you cherrypicked. This PR can be closed, I would rather not submit PRs against the firmware until I have the real thing in my hands. I've compiled my other PR against Linux (in docker, Alpine and Buster amd64)
I saw there was an open issue, so I thought I'd fix it.
BTW, I think I uncloaked an actual bug during this work. The dayofyear value (used in a few places) could be bigger than a byte, but we're coercing it to byte, so we'd better do the same thing we've done elsewhere and send LSB first. I have done that and only hope that the other side is also expecting the same set of bytes. If it isn't there's still another bug needs to be fixed.