Skip to content
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

Internal Errror #68

Merged
merged 25 commits into from
Jun 8, 2024
Merged

Internal Errror #68

merged 25 commits into from
Jun 8, 2024

Conversation

bkbartk
Copy link
Contributor

@bkbartk bkbartk commented May 3, 2024

add internal error message

Close #66

@bkbartk
Copy link
Contributor Author

bkbartk commented May 29, 2024

@TillFleisch
would you please be so kind to review my PR?
I know there are a lot of commits, but in the end the change is rather small.

@TillFleisch
Copy link
Owner

Will do. I'm currently very time constraint, therefore a little patience will be necessary. (This is one of my free time projects and therefore it's not that high on my priority list :/)

After a quick glace I can say the following:

  • The internal error state constants contain a trailing white space, they should not
  • Is there a reason for moving the warning if-block?
  • Were you able to verify this is working as intended on your machine?

@bkbartk
Copy link
Contributor Author

bkbartk commented May 29, 2024

thnx,

  1. I removed the trailing space, Didn't notice that you. You might want to check the German and Italian translations, those come from Google Translate.
  2. the reason here is that for me it makes more sense to catch the warning light first. if for some reason the warning turns on icw any other light a warning is shown instead of the status of the other light.
  3. Yes I was and it worked, After that I did some cleaning of the whole mechanism including the brewer and till now the error didn't show anymore. So I was able to test but I won't be able to test it again.

@TillFleisch TillFleisch merged commit 78563fb into TillFleisch:main Jun 8, 2024
7 checks passed
@TillFleisch
Copy link
Owner

Thank you @bkbartk for adding this! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'Water empty' shown when there is an actual error
3 participants