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

various Fixes #167

Merged
merged 10 commits into from
Oct 17, 2023
Merged

various Fixes #167

merged 10 commits into from
Oct 17, 2023

Conversation

SimonFischer04
Copy link
Contributor

@SimonFischer04 SimonFischer04 commented Sep 30, 2023

This PR includes various fixes / improvements:

@SimonFischer04
Copy link
Contributor Author

@DutchmanNL would you mind taking a look?

@DutchmanNL
Copy link
Contributor

@DutchmanNL would you mind taking a look?

thank you remaindering me, WoW !!! Awesome dude, you ROCK !
I was planning to take this one up (integration of the new API) but didn't had the time yet and just noticed your amazing PR which fully blowed me away!

I will review and test it this week but it looks already awesome and safes me a lot of stuff on my side

Are you reachable on discord or Telegram to discuss if any questions occur or should we keep it here

@SimonFischer04
Copy link
Contributor Author

I'm technically open to dicussing on telegram / discord.
But why not keep it here, so it's also visible for others / documented here and easier to directly comment on specific changes if required, .... ?

@DutchmanNL DutchmanNL merged commit 82b9aa4 into DrozmotiX:main Oct 17, 2023
12 checks passed
@SimonFischer04
Copy link
Contributor Author

Thanks for reviewing and merging. Please create a new release also.

@DutchmanNL
Copy link
Contributor

Thanks for reviewing and merging. Please create a new release also.

Thank you for your input, appreciated!

I didn't see any strange thinks and indeed solution you made with separate state creation fits design of the structure.

Running some test now at my home installation and would like to solve the spam errors if a device disconnects and than I will release a new version to NPM

I expect that will be somewhere end of the week.

Is it ok for you if I include your name for the fixes into the readme ? You deserve the credits!!

@DutchmanNL
Copy link
Contributor

@SimonFischer04 hmm looks like we have an issue, at least on my side. The ESPHome dashboard proces is not working anymore, did you encounter any issues with that ?

@DutchmanNL
Copy link
Contributor

Screenshot 2023-10-18 at 15 38 53

@SimonFischer04
Copy link
Contributor Author

I expect that will be somewhere end of the week.

No problem. Just thought (and was also bit surprised you supposedly did so quick) you already tested as you already merged. Would have been also possibly to test by directly installing the adapter from my branch in iobroker.

@SimonFischer04
Copy link
Contributor Author

Is it ok for you if I include your name for the fixes into the readme ? You deserve the credits!!

Yes, would be pleased.

@SimonFischer04
Copy link
Contributor Author

SimonFischer04 commented Oct 18, 2023

Screenshot 2023-10-18 at 15 38 53

as for one of this logs see: #170 :)

other than that just tested (as already before) again using the direct install from url feature on my test-vm using the new changes on main branch here and dashboard works for me.
Worth to mention that I just found I also (still after fixing #170) sometimes get this log entry. Debugging it, it seems like just a line-break. I didn't change anything about this logging logic here. I don't know why something (only a empty newline) is getting logged here. I think this can be just filtered out?:
image

Do you mean that it actually does not work for you or just the log-entry?

@DutchmanNL
Copy link
Contributor

Do you mean that it actually does not work for you or just the log-entry?

It was not working for me at all anymore (Adapter yes but dashboard not)
Did setup 4 new environments and was not able to reproduce it at all for now except my own installation

will deep dive a little more into it and try to reproduce. Maybe it’s indeed just my installation for some reason

@SimonFischer04
Copy link
Contributor Author

Did setup 4 new environments and was not able to reproduce it at all for now except my own installation

so you just get a 404 when trying to access the dashboard on you own installation?

@SimonFischer04 SimonFischer04 deleted the fixes branch November 1, 2023 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment