-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/UI/countdown #6
Conversation
…a into feature/ui/countdown
Did we test this on a hardware level? |
LGTM, tbh. I am a little worried about the static IP though, can this conflict with DHCP? I mean, usually the router provides IP addresses.. |
Static IP - yes, but I didn't tested a refactored firmware code for now.
In my case, it works 99% of the time and simplifies everything. Sometimes, though, there can be issues with IP availability, like if you frequently power off the device. We need some kind of fallback option, but to implement that, we have to test everything on the hardware, which I currently can't do. An alternative might be a discovery mechanism, but that's a separate task. I think this is a good temporary solution. Plus, it's optional. Should I add a TODO in the code regarding the tasks mentioned above? Or maybe create an issue in the tracker? |
Overall, 99% of the minimal UI is ready. I've also made several edits to the firmware. However, I haven't tested everything on the device yet, so the PR should NOT be merged. Let's discuss the code first; there might be some remarks, etc. Only after testing the firmware on a real device should we proceed with the merge.