-
Notifications
You must be signed in to change notification settings - Fork 21
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
New webpage #63
New webpage #63
Conversation
Looks really good, great job! A couple of thoughts:
In my mind the maybe a good order for the info would be:
|
This looks great! To go along with what @edrikk mentioned with regard to having multiple devices, how about including the hostname and/or using it in the URLs? The current firmware sets it to "Garage Door 123456.local" which makes it cumbersome to use because of the spaces in the hostname, but it does work and it will work across DHCP address changes. I've changed mine to |
Thanks for comments from @edrikk and @radford. I think we will have opportunity to refine the actual look of the page so I'm not too concerned for now in what order things appear. As for function, I have changed the title to match the device name reported to HomeKit (so now "Garage Door ABCDEF") and added a Up Time field which could be helpful (I am experiencing random reboots). Before this is ready to merge, we need to decide on whether to use Async Web or not. Which may be dependent on moving from Arduino to Espressif HomeKit modules. I'll await @thenewwazoo input on that. Thanks |
@dkerr64 Would you update |
Yes I plan to look into that. |
Here is a |
@dkerr64 you should add the ratgdo image somewhere: |
Given @jgstroud’s feedback on the maintainability of the Arduino HomeKit library, I’m quite keen to move away from the Arduino platform. I’ve been (and still am) on vacation and so not spending time coding, but will be picking that back up soon and expect to make good progress given my current experiments’ success. |
I agree that the Espressif platform is likely to be more robust. Kind of scary that it may be a large change. I'll take a look and see what they have for a web-server. |
Implemented.... |
I'm not sure how much effort I want to put into this until we know if/when we move to Espressif platform. While I have not investigated, I suspect that OTA firmware update method might change. What I would like to do is check to see whether a newer version of firmware is available and highlight that. Does anyone know how I can find latest GitHub release number? a URL like https://github.com/ratgdo/homekit-ratgdo/tags takes me to human friendly page of all releases, but how can I get a simple "v0.8.1" or similar string for latest release? |
This looks great. I was wondering why you have the duplicate stats for the door and lock states |
In theory: https://docs.github.com/en/rest/releases/releases?apiVersion=2022-11-28#get-the-latest-release https://api.github.com/repos/ratgdo/homekit-ratgdo/releases/latest <-- doesn't work yet, should when releases are no longer pre-release. |
The HomeKit spec allows for more than just open/closed and locked/unlocked. Doors can also be "opening", "closing", "stopped" and locks can be "jammed" and "unknown". I set the true/false based on just looking for the one value, so the doorOpen value may state false, when the actual state could be opening/closing/stopped. So I include both so user has full visibility. However... I only display the interger value. I should probably convert the number into text. |
I decided to simplify the display.... |
I need a simply way of just getting the version number of latest release. Can that be done? |
Hummm.... so it appears that downloading updates from GitHub requires SSL. And this package accomplishes that by requiring a file of all the valid root certificates recognized by Mozilla. There is a python script that downloads them and creates a certs file... it is 185KB in size. The certificate code requires that to be installed/uploaded into the ESP's filesystem... something I've been trying to stay away from (see the code in this PR that binds the www files into the code binary). So, I could do the same with this certs file... bind it into the binary, then on initialization dump it to the filesystem so that I can pass in the filename to the certificate code (I don't see any way to just pass a pointer to already loaded data). But that will grow the binary size by at least 185KB. Or, the certs file could be stored somewhere and the code could download it to the filesystem on first initialization. But the source server for the certs file probably cannot be GitHub... because catch-22, you need the file for SSL, and GitHub requires SSL. Maybe there is a workaround. Could we just use one self-signed certificate and access GitHub with Any ideas, suggestions? |
When I access GitHub's website and examine the certificate, it is using a DigiCert Global Root CA with expiration date in 2031. That certificate is included in the bundle from Mozilla. I believe we could just extract that one certificate. Assuming that all GitHub.com certificates use that as the root certificate of trust then we're probably okay. If it turns out that others are used we could selectively add them. |
Just using this thread to log progress... ran into a compile error with ESP_OTA_GitHub which will need to be fixed if we want to use this package... yknivag/ESP_OTA_GitHub#15 |
Finally got around to testing this on one of my units, and it looks great. You even added the little icon in the tab bar. Very cool. Also, I may be easily impressed when it comes to web design as it is something I just have not aptitude for. |
Progress report... I am able to create the certificates file, I had to reduce down to just the DigiCert root certificates (for now I have all 13 of them). Including all generated an error message during compile... not enough space. I am able to initialize the ESP_OTA_GitHub, but I am not able to connect to GitHub's URL. Error is at this line. So, something somewhere is not right yet. |
Thank you. Yes I think it is quite good. I'd like to get this ready for merging but I keep trying to add more. The OTA update from GitHub may be a bridge too far. I think I should clean up what I have (I'll remove the unused code I was testing for Async Web) and request merge. Then can tackle some of the more difficult pieces from that base. |
Also, the favicon came from here... it is free to use, but they ask for attribution which we would have to put into bottom of our readme. This is what they ask for... Paste this link on the website where your app is available for download or in the description section of the platform or marketplace you’re using... |
Maybe as a temporary stop-gap, you could just add a link to the releases page. https://github.com/ratgdo/homekit-ratgdo/releases |
@dkerr64 Do you have a favicon set? |
Yes, I am using the one from link in my post above. |
I was originally going to suggest avoid implementing this feature via C++/on-device for slightly different reason: the risk of cipher depreciation 5-10 years down the road (within the expected lifetime of RatGDO device). I held off because in theory a firmware update could/should be released to address this, but as you encountered that might run into space constraints for updated implementation. Suggestions to get this shippable:A) Implement it client side - on page load, run JavaScript function that checks and updates/checks the field with latest. I don't typically recommend this but in this case this does offload all the heavy lifting of SSL/maintenance to the client's web browser. And it should nicely fit into the envelope you are working in - the HTML. B) See what, if anything, other ESP-based or IOT projects are doing. Down the line an improvement/clever way might be found that could implementing this functionality server side. IOT is a thing here to stay so its plausible that non-SSL based endpoints get added to accommodate these micro devices (by Github API or by project itself). If memory serves there might be a way to do either a mock root cert (essentially accept all sites as true) or something like DANE - DNS-based Authentication of Named Entities. The relatively good news is this isn't a novel problem and certainly another IOT project has already tackled this, even SSL.com mentions some options (but don't know enough if they would be a fit within this device/project):
|
Yes, that is a much better idea... do it in the client browser with javascript. Thank you. |
becfe5f
to
4a13076
Compare
@thenewwazoo @donavanbecker @jgstroud I have marked this as ready for review/merge. I'd like to get this into the main line before I try and tackle checking for new firmware version. Thanks. |
You have conflicts. Can you rebase off of the current main branch |
The PR says branch has no conflicts. I never have much success with rebase. |
platformio.ini
Outdated
@@ -11,7 +11,6 @@ | |||
[env] | |||
monitor_speed = 115200 ; must remain at 115200 for improv | |||
board_build.filesystem = littlefs | |||
upload_port = /dev/ttyUSB0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Squash this commit with the "New web page" commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get errors every time I attempt that. The problem (I think) is that I merged main into this branch after the "New web page" commit. I can squash it with the apple-touch-icon commit. But that feels somewhat ugly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First fix the conflicts
make sure you sync the main branch on your fork. then on your branch do a:
git rebase main
then fix the 2 conflicts
then to do the squash on the platform.ini file you can do a:
git rebase -i main
move this commit right after the "new web page" commit and change it to squash
or you could just amend the original commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a pr on your repo showing it corrected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want, we can just merge that one. The commits retained their original authors
remove upload_port = /dev/ttyUSB0 from ini file
I think I got it squashed now. Thanks for the help with git rebase. |
Yep, looks good now. |
This is a work-in-progress so not ready to merge, but I am opening this PR now because 1) I will be traveling for a couple of weeks and 2) I wanted to push what progress I have made towards #56 and get comments.
There are several things going on here.
web.cpp
file and placed into./src/www
directory. A new pre-build script is added to compress them and generate a.h
file to include in the cpp file. This is very efficient and better than storing in the filesystem.status.json
file is for test purposes only... it allows me to us VSCode live server to test all the html..