Time-to-close and further memory improvements

dkerr64 commented 7 months ago

fixes for issues #60 (time-to-close delay) and #143 (only reboot for settings change when necessary). Also includes cleanup and removal of unnecessary code / html pages.

Added implementation for #150

jgstroud commented 7 months ago

at @dkerr64 can we move that last commit for the OTA to a separate PR?

dkerr64 commented 7 months ago

at @dkerr64 can we move that last commit for the OTA to a separate PR?

Yes, done. PR #153

dkerr64 commented 7 months ago

I rebased onto latest main. Sorry for several force-push. Had several conflicts in web.cpp to get right.

I've been running with a browser open for 17 hours now, with server logs going to javascript console. All good.

I was originally thinking of having a separate URL for a separate browser to connect to, and for messages to appear in that browser. But when I looked into it, I decided that camping onto the existing server-sent-event code was easiest and most efficient on ESP memory usage, and just dumping to the browser's javascript console was much simpler than anything else. The server side code is very efficient. Just don't put any RINFO() inside the code that processes it, else infinite loop!!

I added user setting to turn this on, that setting is local to the browser (localStorage) and I hide it if you are on mobile device as makes no sense to send logs to those.

Last piece is to change HomeKit library to capture its logs.

jgstroud commented 7 months ago

The rebase and force push is fine. I haven't had time to test this out yet. Sounds like the new memory layout option isn't showing any negative effects yet. How is your free heap looking? Can multiple clients be monitoring the serial log?

dkerr64 commented 7 months ago

No negative effects. But we will have to keep an eye on things. Free heap is in good condition.

As for the IRAM heap. We need to keep track...

json string buffer... 1024 bytes.
log lineBuffer... 1024 bytes (could probably reduce to 512 or less, for sprintf() into for each long line.
log msgBuffer... 2048 bytes (holds about last 20 log lines, could increase to 4096?).
savedLogs... 2048 (malloc's and free'd to read saved log file into when display requested).

In other words, we need at least 2x the message buffer space as we must allocate space to read the saved file into. Now in theory I could probably read the file in chunks, but it would get rather complicated because the "head" of the circular buffer could be anywhere in the file. So long as we have the space, I prefer reading it all in at once.

jgstroud commented 7 months ago

We can log the free IRAM heap as well, but maybe not add to web ui

https://github.com/esp8266/Arduino/blob/master/libraries/esp8266/examples/irammem/irammem.ino#L465

dkerr64 commented 7 months ago

We can log the free IRAM heap as well, but maybe not add to web ui

https://github.com/esp8266/Arduino/blob/master/libraries/esp8266/examples/irammem/irammem.ino#L465

I don't think we need to. It is very stable, we only malloc on initialization. I just removed the dynamic malloc/free when reading from the saved file and instead malloc that buffer on initialization like the rest. So free space will never change during runtime.

dkerr64 commented 7 months ago

I added HomeKit library logs to our custom logging.

This is ready for a thorough thrashing and subsequent merging. I don't have anything left on my to-do list for now.

dkerr64 commented 7 months ago

@jgstroud just remembered... do you want to put back in the setTimeout(500). I'll leave that to you to decide.

dkerr64 commented 7 months ago

And also... you might want to implement a rolling code write-to-flash minimizing solution. If you do, suggest you write it on crash callback and on user requested reboots.

jgstroud commented 7 months ago

Just leave it as is for now. I did a quick review of the code. Once I get a chance to test it, we can merge. But I'll probably want to run this code for a week or so before we release with it.

jgstroud commented 7 months ago

And also... you might want to implement a rolling code write-to-flash minimizing solution. If you do, suggest you write it on crash callback and on user requested reboots.

Yes I definitely want to add this before we release.

jgstroud commented 6 months ago

I've rebased, reordered some of the commits and squashed some commits. I don't think I can update this PR, so I'm going to replace this PR with a new one

dkerr64 commented 6 months ago

Do you still want the Keep track of last door state change commit without the NTP client

Yes please, I'd like to keep that. I removed all the NTP client so it just tracks it from last reboot. I find it useful and its minor stuff.

dkerr64 commented 6 months ago

I've rebased, reordered some of the commits and squashed some commits. I don't think I can update this PR, so I'm going to replace this PR with a new one

Yes, definitely some squashing can be done. Thanks. I can make you a collaborator (thought you already were) or you can do a new PR if you prefer. Your choice.

jgstroud commented 6 months ago

I've rebased, reordered some of the commits and squashed some commits. I don't think I can update this PR, so I'm going to replace this PR with a new one

Yes, definitely some squashing can be done. Thanks. I can make you a collaborator (thought you already were) or you can do a new PR if you prefer. Your choice.

I was able to update the PR. Thanks. It could probably be cleaned up more, but for the most part, I'm happy with this

jgstroud commented 6 months ago

I never moved the crashlog from eeprom to filesystem. It started to seem like a big rewrite of the utility and I'm not sure we gain a whole lot from it. we can do that later though

dkerr64 commented 6 months ago

I never moved the crashlog from eeprom to filesystem. It started to seem like a big rewrite of the utility and I'm not sure we gain a whole lot from it. we can do that later though

I'm fine with that. What about overwriting previous crash data, so it only attempts to keep the latest? Save that for future too?

dkerr64 commented 6 months ago

I've rebased, reordered some of the commits and squashed some commits. I don't think I can update this PR, so I'm going to replace this PR with a new one

Yes, definitely some squashing can be done. Thanks. I can make you a collaborator (thought you already were) or you can do a new PR if you prefer. Your choice.

I was able to update the PR. Thanks. It could probably be cleaned up more, but for the most part, I'm happy with this

good, thanks. Embarrassingly I still have that missing semicolon commit visible to all !!! But I suppose that demonstrates integrity and that I'm not ashamed to make my trivial errors visible. :-)

jgstroud commented 6 months ago

I've rebased, reordered some of the commits and squashed some commits. I don't think I can update this PR, so I'm going to replace this PR with a new one

Yes, definitely some squashing can be done. Thanks. I can make you a collaborator (thought you already were) or you can do a new PR if you prefer. Your choice.

I was able to update the PR. Thanks. It could probably be cleaned up more, but for the most part, I'm happy with this

good, thanks. Embarrassingly I still have that missing semicolon commit visible to all !!! But I suppose that demonstrates integrity and that I'm not ashamed to make my trivial errors visible. :-)

oops, missed that one

ratgdo / homekit-ratgdo

Time-to-close and further memory improvements #145