chiefwigms / picobrew_pico

MIT License
149 stars 63 forks source link

Update Brew Session Graphing (error/pause states) #250

Closed tmack8001 closed 2 years ago

tmack8001 commented 3 years ago

I've tested this with manually created files generated from a prior brew session, I don't know how to trigger all the different error conditions on the device but I'm certain that the device only sends int types for PauseReason and ErrorCode so tracking those as int for now. While only "translating" the ones that we know about as we detect them... Right now the main one is PauseReason: 1 which is when you program a pause (or the recipe ends). I'm not positive that the error codes listed on the support site match 1-1 for the error codes in the API.

image

image

tmack8001 commented 3 years ago

In February I tore apart the brewing room... now that things are "mostly" put back together I will be getting back into brewing again and thus will have more time to test and button up this feature.

tmack8001 commented 2 years ago

I'll be running a session this weekend with a pause step to connect a chiller during the last 5 minutes of the "boil". Will verify that this all works as intended for that, porting to other device types can happen later since I don't have one of these myself that is functional and that would require getting an error condition to trigger given there isn't a programmatic "pause" for the smaller devices.

tmack8001 commented 2 years ago

In running a few sessions this week there are a few glitches in the coloring and status of the graph. I'll work on these and update when ready.

wiltdavi commented 2 years ago

If you have anything on this you want tested I am brewing tomorrow with my Z and I have both a manual pause and pause step in my brewing procedure.

Sorry for the late notice.

tmack8001 commented 2 years ago

oh thanks for the offer @wiltdavi when I ran an actual session with this with 2 pauses I noticed I was doing something wrong when detecting the width of the pause (combining earliest with the latest) and messing up the view. I'll work on this when I find/make time this week and improve it before calling it ready for merge.

Fixed a few issues that I have found so far:

tmack8001 commented 2 years ago

This PR is complete and I tested various different states. The one thing that does get odd is with multiple pause states that overlap next to each other you can't really read them... but same is true with step names so not making that a problem to solve right now.

Like this, I don't have a C/S/Pro or Zy that is functional right now nor know the contract for those machines well enough to know if the local pause state from a user is send server side or if the final brew complete signal is sent in a similar way.

@wiltdavi if you want to take this for a spin go ahead and git pull down my branch here to test and play around with on your own. I eventually want to make "beta" testing these types of changes easier say a textfield in the /about screen when navigated to with /about?mode=dev or something so you can select the remote branch (allowing for a direct source branch or even a forked repo's branch) to be pulled and compared against. Could also be as simple as a remote.url and remote.branch in the config.yaml as well 🤷‍♂️ .

wiltdavi commented 2 years ago

@tmack8001 sorry for the late response. I have taken a look at this new feature and have a few questions.

  1. I updated to this code and looking at my Brew History I was not able to see any of the pauses which I have in my process. Looking at the code it appears that you are now logging new information, correct? If this is the case then I will not see the new pause information until I run a session with this code, correct?

  2. After updating to this code I reverted back to a previous version, for my investigation for #318 and #321, and when I go back to this older version the Brew History graph is no longer shown. I was thinking that this was due to a file change you must be doing to support the new pause information but I cannot find any modifications to my session files nor does over righting the sessions with a backup I had restore the graphs. This is not really an issue as when I switch back to the latest version of code the graphs are view able again but I am trying to understand why this behavior is occurring.

image

tmack8001 commented 2 years ago

1 yes only sessions run with this code will have pause or network status information logged in the session files.

2 that graph not appearing might be a bug in this branch that is impacting that due to the graphing data changes I made and how this was revised with the more general graphing centralization effort I did. Will check it out.

tmack8001 commented 2 years ago

checkout the fix I have done in https://github.com/chiefwigms/picobrew_pico/pull/323 for this specific issue

wiltdavi commented 2 years ago

@tmack8001 I am so stupid. The reason that I was not getting the graphs was the brew_graph.js file was being cached in my browser as I was not closing it throughout my testing. Due to this, I am not sure #323 is required unless you found a reason for it.

tmack8001 commented 2 years ago

I was able to reproduce locally and thus the changes made in the mentioned PR. Likely when you were going back and forth between the git shas JavaScript was being cached in your browser and causing there to be odd behavior.

Specifically the session graphs after "load more" (say session index 10+, where first session is 0 indexed) aren't working due to the dynamic loading of html via innerHTML of which the HTML5 specification doesn't execute any included JavaScript (aka what is rendering the graph from the data points).

wiltdavi commented 2 years ago

Well glad you found something based on my comment.

I will take a look at this new feature next time I run a session with pauses in it.

tmack8001 commented 2 years ago

@wiltdavi all Z recipes have a end of session pause regardless if you program one in or not.

Also note that this only impacts Z model devices.