lbussy / keg-cop

A WiFi-enabled keg and tap list solution.
https://www.kegcop.com
MIT License
19 stars 4 forks source link

Allow Invert of Cooling Pin #19

Closed austinpe closed 2 years ago

austinpe commented 2 years ago

Currently the cooling only works when wired Normally Open (it says so in the docs to do this 😄).

However, some people don't feel comfortable building their own high voltage relays so opt for off-the-shelf products (e.g. powerswitch tail from adafruit or the IoT Relay from Digital Loggers).

As such, it would be nice to allow inverting the Cooling signal to be wired as Normally Closed.

Looking at code, I think it's as simple as turning the HIGH/LOW argument of the digitalWrite function into a variable within the thermostat.cpp.

I'll try to do this update myself and submit a pull request.

Thoughts?

lbussy commented 2 years ago

Hi Austin. I don't have an issue with this in theory. I looked at the device you are talking about and it looks like it's configurable. Is that not the case?

My only concern here is if you have a failure of the controller, your cooling system will stay on.

lbussy commented 2 years ago

Notes for myself; pin state is set in (devel):

https://github.com/lbussy/keg-cop/blob/5b3bd46a5df5442c4f9e612c788a3069c134fb5a/src/main.cpp#L78 https://github.com/lbussy/keg-cop/blob/5b3bd46a5df5442c4f9e612c788a3069c134fb5a/src/thermostat.cpp#L51 https://github.com/lbussy/keg-cop/blob/5b3bd46a5df5442c4f9e612c788a3069c134fb5a/src/thermostat.cpp#L80 https://github.com/lbussy/keg-cop/blob/5b3bd46a5df5442c4f9e612c788a3069c134fb5a/src/thermostat.cpp#L105

lbussy commented 2 years ago

Added config.temps.coolonhigh in the json.

austinpe commented 2 years ago

You are correct that the device is configurable, but the layout is not. For example, on the IoT Relay if you plug into the NO socket then you block the 'Always On' socket. Not really your project's problem, but part of my motivation for the request!

And yes, using the Normally Closed relay would result in a 'failing closed' situation if the controller failed and you'd be cooling when you shouldn't be. I think this may actually be a preferred result for some, though.

For example, the chest freezer I use for my keezer has a dial that allows the user to set how cold/'hot' they want the chest freezer to be by default. I have it turned all the way up such that without any temp control device the freezer barely gets below freezing (but still too cold for beer 😄). The keezer is in my garage, so on a hot summer day if the controller fails open and I don't notice it I'd rather open the top up and let it 'warm up' ~10 °F than wait for it to cool down by ~10 °F.

lbussy commented 2 years ago

I get ya. Okay, I mean this is will be a "change at your own risk" setting of course. I have it coded, I just need to add a web page form field and a handler for it.

lbussy commented 2 years ago

Added a POST handler:

            if (strcmp(name, "coolonhigh") == 0) // Enable cooling on pin high (reverse)
            {
                if (strcmp(value, "true") == 0)
                {
                    Log.notice(F("Settings update, [%s]:(%s) applied." CR), name, value);
                    config.temps.coolonhigh = true;
                }
                else if (strcmp(value, "false") == 0)
                {
                    Log.notice(F("Settings update, [%s]:(%s) applied." CR), name, value);
                    config.temps.coolonhigh = false;
                }
                else
                {
                    Log.warning(F("Settings update error, [%s]:(%s) not valid." CR), name, value);
                }
            }
lbussy commented 2 years ago

Okay, @austinpe: I don't have a testbed where I am right now, but I have created a cool_state branch which should do what you need. Please let me know how it works for you.

austinpe commented 2 years ago

awesome! I'll test it within the next few hours. Is it possible for me to do an OTA with the new firmware in that branch? Or will I need to flash it manually?

lbussy commented 2 years ago

You would have to do it manually, via PlatformIO if you have it. If not, I can try to put it up on BrewFlasher for you.

lbussy commented 2 years ago

It's in BrewFlasher. You can use the app, or BrewFlasher Web:

image
austinpe commented 2 years ago

The webapp didn't have an error message, but it "flashed" much too fast to be real:

Serial Out of the 'flashed' controller shows it's in a bootloop, just keeps repeating this message:

rst:0x3 (SW_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT) configsip: 0, SPIWP:0xee clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00 mode:DIO, clock div:2 load:0x3fff0018,len:4 load:0x3fff001c,len:1044 load:0x40078000,len:10044 load:0x40080400,len:5872 entry 0x400806ac ets Jun 8 2016 00:22:57

In BrewFlasher standalone:

Verifying firmware list is up-to-date before downloading... Downloading firmware... Downloading partitions file... Error downloading partitions file! Error - unable to download firmware.

I see you put the firmware in the branch. I'll flash manually with esptool.py

lbussy commented 2 years ago

Sorry about that - I'm coding blind here. I did fix my build automation so if those versions do not work please let me know.

lbussy commented 2 years ago

Just now (18:14 CT) I pushed a new firmware. I am not sure but I think I may have uploaded the wrong one. Try again if you already flashed the other version.

austinpe commented 2 years ago

I grabbed the latest firmware from the repo. I cleared the esp32 first. I flashed the partition, firmware, and spiffs manually with esptool.py to the correct address according to the docs.

There's a different bootloop now:

rst:0x10 (RTCWDT_RTC_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT) flash read err, 1000 ets_main.c 371

Not sure if you updated BrewFlasher but it still says there's an error with downloading the partitions file.

lbussy commented 2 years ago

Grrr ... sorry about that. I need to find an ESP32 here somewhere to try this on. Will be morning (not sure where you are, about 12 hours from now) before I can get to it. The changes were simple enough that I didn't expect a crash.

austinpe commented 2 years ago

Absolutely no problem. Thank you for your time and energy on this it's beyond appreciated. I'm east coast US, btw.

If there's a clear starting point you see I can poke around tonight. I've used PlatformIO before so can compile the source myself, but I would not say I'm fluent in C++... enough to be dangerous.

lbussy commented 2 years ago

My next step would be to compile in debug mode and use the exception decoder to figure out where it's crashing. It's never straightforward. :)

lbussy commented 2 years ago

I was also talking to @thorrak about adding Kasa plug support. Would that be interesting to you at all? Even less wiring that way.

austinpe commented 2 years ago

100% -- and I'm happy to help test/code. I actually commented on the Fermentrack thread on HBT (I'm 'Feynman' over there) saying that I was excited about that feature.

I actually had my own make-shift Kasa setup. I monitored the cooling signal using an always-running python script and used it to turn on/off the Kasa plug. But it just wasn't sustainable. But, I have several Kasa plugs and Kasa strips already that I used for my testing so I'm happy to help if I can!

The risk I've found from the Kasa plug strategy is that if there's a power surge or random network outage then it's really hard to have robust recovery code that handles all cases. Whereas a hardwired setup can survive network outages. I know technically Kasa should be able to communicate via local network even if there's no internet, but I found in my testing that it wasn't dependable.

lbussy commented 2 years ago

Ultimately, it's going to be up to the end-user to come up with a robust networking strategy and that may include some work around the order by which things recover in case of an outage. I know I have to do that myself (and Kasa plugs are part of that.) IFTTT is one way a person can do it.

austinpe commented 2 years ago

oh for sure, I agree. But either way, yes, I think it would be a great option to have and I'm happy to help with it.

lbussy commented 2 years ago

Okay, I seem to have broken BrewFlasher with my update. Till Thorrak has a chance to tell me what I did wrong, you can download the firmware here:

1.0.1-Alpha.1 Allow Inverted Cooling Pin

This is tested in that I ran the interface and tested the interaction of the web page with the configuration. I do not have a physical testbed here where I can verify the pin action, so please be aware of that.

The boot loop crash you got was a product of something BrewFlasher did with the screwy firmware settings I added. You will have to do a full erase and then flash these binaries. After that of course you will have to set your wifi and all that just like new.

austinpe commented 2 years ago

ok, good news and bad news:

It seems that the invert works! I'll keep monitoring.

By the way, did the deadband range for turning on/off the cooling change? Maybe it's just because I'm watching it, but it feels like it's cycling more often? There's no 'short delay' implemented now is there? Again, probably just because I'm actively watching it.

Bad news: It seems like the ability to update the taps is broken. I can select a tap and fill in the form, but clicking on 'update' does nothing. Update: I can successfully update the tap info via API

By the way, is there a way to remotely monitor the serial out so I can debug while the esp32 is installed?

lbussy commented 2 years ago

It seems that the invert works! I'll keep monitoring.

I approve of this message.

By the way, did the deadband range for turning on/off the cooling change? Maybe it's just because I'm watching it, but it feels like it's cycling more often? There's no 'short delay' implemented now is there? Again, probably just because I'm actively watching it.

I didn't mess with that part - I'll have a look.

It seems like the ability to update the taps is broken. I can select a tap and fill in the form, but clicking on 'update' does nothing. Update: I can successfully update the tap info via API

I think I know what that is. I blame @thorrak for it. Labels or something. I'll take another look at that part.

austinpe commented 2 years ago

AH! Yes, so I went back via the web interface and put a number into the 'Tap Label' and now it updates. There's a 'placeholder' value of 1 so I thought it was already filled in when I tried it earlier.

This is what it looks like as the page loads. The 'Tap Label' value is not populated and it's just a placeholder value. You can see how someone could easily think that the 'tap label' value was already set: image

So, even after the tap label is set once, it doesn't populate in on refresh so you can't make changes until you enter something again.

lbussy commented 2 years ago

There's a 'placeholder' value of 1 so I thought it was already filled in when I tried it earlier.

Yep, there's still a bug on my side. I need to fix that.

austinpe commented 2 years ago

Regarding the cycling time, it looks like the COOLDELAY variable within config.h is the same between branches.

However:

// Cooling Delay - Default 5 mins (in millis())
//
#ifndef COOLDELAY
#define COOLDELAY 2 * 60 * 1000
#endif

It's set to 2 minutes instead of 5 minutes 😄

and

// Minimum on time - Default 2 mins (in millis())
//
#ifndef MINON
#define MINON 1 * 60 * 1000
#endif

MINON is 1 minute instead of 2

lbussy commented 2 years ago

Heh, I wonder what I was thinking there.

You beat me to it, I was going to compare that after I finished re-re-re-fixing my build script.

I can set those back to the initial values here in a moment and give you Alpha.2.

lbussy commented 2 years ago

BrewFlasher is still broken, you can update with the firmware here:

1.0.1-Alpha.2 Allow Inverted Cooling Pin (Cleanup)

To recap:

You should be able to verify the version in the "About" page.

Let me know!

austinpe commented 2 years ago
  • In theory I have not broken the cooling pin inversion logic

✔️

  • I fixed tap labels

✔️

❌ -- I put a comment in that bug

  • I reverted some changes to the min cool time as well as delay

✔️ -- well at least I trust that this is done 😄 . I'll keep my eye on the logged coolstate changes!

Awesome work! 🥳

Anything else I can do? I'm happy to keep testing -- I have a spare esp32 sitting here

thorrak commented 2 years ago

Okay, I seem to have broken BrewFlasher with my update. Till Thorrak has a chance to tell me what I did wrong, you can download the firmware here:

BrewFlasher should be back working now. Sorry about that!

thorrak commented 2 years ago

I actually had my own make-shift Kasa setup. I monitored the cooling signal using an always-running python script and used it to turn on/off the Kasa plug. But it just wasn't sustainable. But, I have several Kasa plugs and Kasa strips already that I used for my testing so I'm happy to help if I can!

The risk I've found from the Kasa plug strategy is that if there's a power surge or random network outage then it's really hard to have robust recovery code that handles all cases. Whereas a hardwired setup can survive network outages. I know technically Kasa should be able to communicate via local network even if there's no internet, but I found in my testing that it wasn't dependable.

Were you using the Kasa Cloud or the local ports with your python script? I've had pretty good success with the local ports, but definitely have had devices disappear randomly on the cloud.

The way that I have it working with BrewPi-ESP is to default to failing "off". I'd have to recheck the timings, but every ~5 seconds or so the controller sends a message to the Kasa switch telling it "shut off in 90 seconds". If the network goes out, controller dies, etc. then it has 90 seconds to come back before the Kasa switch shuts off whatever is connected.

austinpe commented 2 years ago

I was using the Kasa python library and directly interacting with the plugs.

I can put my code in a gist if you're interested, but I don't think it's anything too special.

Basically it monitored the faux LCD from Fermentrack (/api/lcd/) to look for the cooling status then syncs that status with the Kasa plug.

lbussy commented 2 years ago

Don't mind me - keep talking. Just closing this issue as it's been moved to devel.