TimoKropp / OPENCHESSBOARD_WiFi

MIT License
37 stars 5 forks source link

Contribution policy #4

Open michaelbaudino opened 9 months ago

michaelbaudino commented 9 months ago

I've been working on some code refactoring with the goal of being able to easily debug the whole code and make it more reliable and resilient (I find it quite difficult to complete a whole game without issues).

To name a few I have on top of my head:

And maybe (hypothetically!) a few improvements later (in the long term, I'd like to let my 5yo child play, thus without a smartphone):

Would you be interested in merging some of those upstream? Should I submit pull requests as I work on that? Or should I rather keep it in a forked project?

I'm a bit afraid that a forked project would quickly diverge to a point of no-return (well, a point of "no-merging-upstream-possible"), but I would also totally understand if you'd prefer to keep the code as steady as possible.

TimoKropp commented 9 months ago

let me get to your ideas one by one:

1. (I find it quite difficult to complete a whole game without issues): A1: This seems to be an issue with the client library which sometimes looses connections without closing and restarting the client. It only gets notices when you are making a move. There are some work arrounds but I am still not sure what is the most robust way to solve this.

2. extracting user config to a separate file: A2: Sure thing, just make a proposal and we will push it to main. I would like to seperate or hide more complex settings which are more likely to mess up the board.

3. DRY and isolate Lichess API code: A3: Also perfectly reasonable, lets do it.

4. bump ArduinoJson to version 7 (and use it as an external library rather than) A4: When using an external library, it would be good to restrict the version to a specified one. In PlatformIO this would be easy, but in the Arduino IDE not so easy. So the most stable solution is maybe to keep small libraries included in the files. But surely we can also move to external library an see when it fails.

5. isolate game engine to a separate file/class A5: Not completely clear what you mean by that. There is currently no real game engine, but a state machine would definitly make sense, especially if you want to include more game selection options like you proposed.

6. re-organizing files, to make the project compilable with PlatformIO IDE A6: This is also a good idea, but keep in mind that most users of the board are beginnsers and are mostly familiar with the Arduino IDE. So the file system should be easy to import into an Arduino Project.

7. improve sync check between our internal board representation and Lichess game status A7: This should in principle be handled by the stream client, but there are some issues when the connection is lost. Maybe there should be a check without polling the lichess API to ensure the connection is stable or needs to be restarted. Keep in mind that polling will restrict the bandwith of your client and could lead to more issues down the line.

As a general guideline for PRs. Make them as small as possible and easy to test. If you want to make bigger changes feel free to make a fork and we can release new versions for specific purpose in parallel to the main branch. If we decide a new branch is more stable then the current master, we can promote the new branch as new master and put the older ones to archive.

Its really great that you want to contribute and have so many ideas and improvements :)

TimoKropp commented 6 months ago

I added support for esp32 and made some small improvements in structure. I further simplified the code and made it a little bit more robust. On esp32 with proper client handling it seems that the client disconnect issue is no longer present.