search

JanLunge / pog

A Kmk firmware flashing and configuration tool
https://pog.heaper.de/
MIT License
326 stars 20 forks source link

11 pog on boards without vbus sense #13

Closed parallacks closed 4 months ago

parallacks commented 4 months ago

Added check for if the vbus pin is even needed and if it's a valid option. Added the pin checker to the renderPin() function. I've tested this with a rp2040-zero which doesn't have a VBUS_SENSE pin. I haven't tested the 'quickpin' version yet.

I'm not a huge fan of this implementation overall because it allows incorrect setups to just quietly fail. If a user puts in improper pin numbers their firmware will run but they'll have a hard time figuring out why their keyboard isn't working. I've added a debug message when we find an invalid pin so if they connect to via serial they'll at least see that. It'll be less of an issue when we add the ability to check validity of pins before even creating the pog.json

Marked as draft as I want to test the quickpin before merging.

Fixes #11

JanLunge commented 4 months ago

I like this implementation a lot, I had the idea that error messages could always be send via serial and pog would autoconnect to the serial port of the keyboard (the second serial is also use for the update when the drive is hidden) and then print any errors directly in the gui. Also I already started a serial command to read all the available pins from the board so that the gui could show hints when typing pin names and show an error even before saving. So I would say this PR is a very good step into that direction.

parallacks commented 4 months ago

I agree its in the right direction. I should've said I don't like this implementation being the only thing checking. Having the checks that you're working on will greatly reduce the amount of times we'll even need these check. And this PR will catch things if someone tries using a shared pog.json.

JanLunge commented 4 months ago

Can i merge this? Seems good enough to me for now

parallacks commented 4 months ago

I was wanting to do some testing with using the 'quickpin' before merging. But I'm looking at the code now and I don't see how it would work even with out my code. Don't you need to import the quickpin module for the particular board you're using for it to work? I don't see that import anywhere.

JanLunge commented 4 months ago

Yes exactly for quickpin you would need to import your board yourself. Did not find a good way to list compatible quickpin boards yet

parallacks commented 4 months ago

Ok I don't think the pinValid() function will play nice with quickpin currently. I think we'd need to eval() the pinLabel first then check it.

parallacks commented 4 months ago

Ok sorry for the delay, I've added some extra code to account for quick pin and I've tested it. Should be good to merge now.

JanLunge commented 4 months ago

No worries, very nice that you added this for quickpin as well.