vonmillhausen / sf2000

Information regarding the SF2000 handheld console
https://vonmillhausen.github.io/sf2000/
248 stars 14 forks source link

SF2000 Battery Meter Fix #9

Closed Dteyn closed 1 year ago

Dteyn commented 1 year ago

I've created a SF2000 Battery Meter Fix tool based on your excellent Boot Logo Changer tool. I have tested this on the current firmware as well as the previous firmware and it seems to be working well for me.

It was made a lot easier thanks to your very thorough and detailed documentation and commenting. It is always a pleasure to work on projects that are so well commented. I've barely used JavaScript before but thanks to your commenting I was able to muddle my way through it without much difficulty.

PS: This is my first pull request ever so I hope I did things correctly! If I messed anything up please let me know so I can improve in the future. Thanks for all your hard work!

vonmillhausen commented 1 year ago

Hi @Dteyn! Apologies for the late reply - I had visitors all weekend, and didn't really have any time to look at SF2000 stuff.

Great job on the battery meter fix tool, and I'm glad you found the comments I included in the tools I wrote helpful! I wrote them not only for other folks who might want to build other tools, or take functions for their own use, but also for myself... there's nothing worse than staring at code you wrote yourself years previously and have no idea of what it is, how it works, or why you did things the way you did 😅

And yes, I believe you did the pull request correctly; truth be told, it's the first pull request I've ever been involved in on either side, so I'm no expert... but from what I can see it looks like you did it perfectly 👍

That said, I'm going to go ahead and close this pull request without merging it. The Github document I wrote, and the tools I put together based on the original button mapping tool by Osaka I have done for my own amusement and education. I'm kind of amazed that this repo has garnered the attention that it has, especially with much more technically impressive and comprehensive tools like Tadpole around. As part of that, I'm just a little uncomfortable with the idea of hosting tools or code developed by other folks in the repo; personally, I feel that having a broader spread of folks writing and hosting tools for the SF2000 is better in the long-run, rather than having one single central place to go... I mean, look what happened last week when Github randomly flagged my account and took down all my repos (they still haven't responded to my support ticket, btw!).

I hope you don't mind, and please don't take offence; the tool you wrote is great, and I hope that you'll continue to host it yourself at https://dteyn.github.io/sf2000/tools/batteryMeterFix.htm (I'm already linking to it there from my main Github page) so that the community can continue to take advantage of it and fix the broken stock firmware power monitoring curve for the stock battery 🙂

vonmillhausen commented 1 year ago

Oh, and I did have one tiny technical suggestion for you. Currently, your tool is based on the boot logo changer, which had a "three step" process:

  1. The user provided a firmware file
  2. The user provided an image file
  3. The user clicked a button to download the patched firmware file

As such, the battery meter tool currently also maintains a three step process:

  1. The user provides a firmware file
  2. The user clicks a button to patch the firmware file
  3. The user clicks a button to download the patched firmware file

But in this case, the user isn't providing a second file - and so theoretically the second step of having to manually click a button to do the patching isn't strictly necessary. My suggestion would be to roll the patching of the provided firmware file's data into either the function that reads and validates the firmware file from the user, or into the download function; that way, you can cut the tool down to a two-step process, which will provide a slightly nicer/faster user experience.

If you want an example of this, I have a another dev-focused tool that's two-step here: https://github.com/vonmillhausen/sf2000/blob/main/tools/biosCRC32Patcher.htm

A bit like the battery meter fix, CRC32 patcher just reads in a firmware file, patches some bytes in it, and then lets the user download the patched version; in this case, it does the patching in the function that reads in the data from the provided firmware file.

I hope that makes sense!

Dteyn commented 1 year ago

Sounds great!

Thanks for your kind words. I appreciated your code commenting greatly, since I program and think much the same way. I know that feeling you mentioned of coming back to a project and not having a clue what I was thinking at the time. The comments help so much with putting yourself back in that mindset.

I understand and appreciate where you're coming from about hosting the tool on your repo. You are doing a fantastic job though, all the information on that page is part of what convinced me to buy a SF2000 in the first place, since I could see it was so well documented and there was a thriving community around it. It's been a blast participating!

I'll continue to host the battery meter fixing tool on my repo. I also appreciate your tips about making it into a 2-step process. Not knowing JavaScript too well, I didn't want to mess too much with the core functionality of the interface and risk something going wrong. With that said, the biosCRC32Patcher seems like a more appropriate starting point as you mentioned. I'll likely revisit this and rework batteryMeterFix so that it uses biosCRC32Patcher as the base.

I really enjoyed working on this and looking forward to what the future holds for the SF2000! Cheers! 👍

vonmillhausen commented 1 year ago

For sure! I always think of comments as being like a time capsule for my brain 🤣 🧠

Thanks for your understanding, and for continuing to host your battery tool yourself! 👍 The froggy sure is a neat device; I've had a lot of fun messing with it, and following along with the amazing folks working on custom firmware for it. It would be amazing if the device can live up to its potential to be a fairly decent SNES, GBA and older retro emulation device... and even if that ultimately never comes to pass, it's still an excellent little Game Boy!

Sounds good re: the battery meter fix tool. One thing to note is that the CRC32 patcher doesn't have any validation on the BIOS version provided to it (it'll literally just patch any file that's at least 520 bytes in size - it'd patch a text document if it was big enough!), so you'd need to make sure to add the validation stuff to the first step if you were going to start with the CRC32 patcher as a "blank" canvas 👍