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

Feature: Several Basic and Advanced Settings, Event logs, Last Unlock User #59

Merged
merged 25 commits into from
Oct 28, 2024

Conversation

AzonInc
Copy link
Collaborator

@AzonInc AzonInc commented Oct 25, 2024

This PR adds several basic and advanced settings entites as well as event support.

Switch:

  • Button Enabled
  • LED Enabled
  • Night Mode
  • Night Mode: Auto Lock
  • Night Mode: Reject Auto Unlock
  • Night Mode: Lock at Start Time
  • Auto Lock
  • Auto Unlock: Disable
  • Auto Lock: Immediately
  • Automatic Updates

Text Sensor:

  • Last Unlock User

Select:

  • Single Button Press Action
  • Double Button Press Action
  • Fob Action 1
  • Fob Action 2
  • Fob Action 3

Number:

  • LED Brightness

Events:
Lock events are sent to Home Assistant with the esphome.nuki identifier by default.

event_type: esphome.nuki
data:
  device_id: 373c62d6788cf81d322763235513310e
  action: Unlatch
  authorizationId: "3902896895"
  authorizationName: Nuki ESPHome
  completionStatus: success
  index: "92"
  timeDay: "25"
  timeHour: "0"
  timeMinute: "46"
  timeMonth: "10"
  timeSecond: "11"
  timeYear: "2024"
  trigger: system
  type: LockAction
origin: LOCAL
time_fired: "2024-10-25T00:46:33.398284+00:00"
context:
  id: 01JB0J7AXPMS5DWHG188Y6XFCP
  parent_id: null
  user_id: null

@AzonInc
Copy link
Collaborator Author

AzonInc commented Oct 25, 2024

I synced the base library with the main. I would have linked it directly to a specific release version but it does not seem like releases are following the code updates.

Using commit hashes works just fine.
For example: https://github.com/I-Connect/NukiBleEsp32#93e7da927171c8973b7ef857c7fa644c174ed47d

Library Manager: Installing git+https://github.com/I-Connect/NukiBleEsp32#93e7da927171c8973b7ef857c7fa644c174ed47d
INFO Installing git+https://github.com/I-Connect/NukiBleEsp32#93e7da927171c8973b7ef857c7fa644c174ed47d
git version 2.39.5
Cloning into '/home/runner/.platformio/.cache/tmp/pkg-installing-2xroo738'...
HEAD is now at 93e7da9 Added MIT license
INFO NukiBleEsp@2.0.0+sha.93e7da9 has been installed!
Library Manager: NukiBleEsp@2.0.0+sha.93e7da9 has been installed!

And the dev builds are failing because the dev branch is still behind.

I changed the lib reference to the base repo and added the last commit hash as branch for the main build and in readme.
The dev build uses the latest version of the base branch from I-Connect.

With this change you don't have to maintain two repos anymore and still have reference to a specific version of the code.

@uriyacovy
Copy link
Owner

  1. No problem using the hash in github actions, but it's less clear when normal users need to define it. That's why I forked the repo.
  2. After using the previous PR, I got some instability (disconnections) for long periods (an hour+). I'll retry now with this new version and update.
  3. I saw that you added delays of 5 sec. Why are they needed? Also, from my experience long delays cause issues with ESPHome and HA.
  4. I think it also worth updating the README with the latest features you've added.

@AzonInc
Copy link
Collaborator Author

AzonInc commented Oct 25, 2024

  1. No problem using the hash in github actions, but it's less clear when normal users need to define it. That's why I forked the repo.

Ah okay, well... so lets do this for actions and still use yours for the Readme part.

  1. After using the previous PR, I got some instability (disconnections) for long periods (an hour+). I'll retry now with this new version and update.

Maybe thats caused by the more frequent request of the config after each status update.
Do you know if it's only blocking the cycle or rebooting due to memory issues?

  1. I saw that you added delays of 5 sec. Why are they needed? Also, from my experience long delays cause issues with ESPHome and HA.

There is probably a better way to achieve this by waiting for a period of time and then check again.
The results are not immediately available.

  1. I think it also worth updating the README with the latest features you've added.

I added all the nee Entites there and also mentioned the Events part. Or did u mean smth else?

@uriyacovy
Copy link
Owner

  1. After using the previous PR, I got some instability (disconnections) for long periods (an hour+). I'll retry now with this new version and update.

Maybe thats caused by the more frequent request of the config after each status update. Do you know if it's only blocking the cycle or rebooting due to memory issues?

I didn't save the logs so I can't tell for now. I follow and update.

  1. I saw that you added delays of 5 sec. Why are they needed? Also, from my experience long delays cause issues with ESPHome and HA.

There is probably a better way to achieve this by waiting for a period of time and then check again. The results are not immediately available.

AFAIK ESPHome is asynchronous, and to achieve this behavior you have to use set_timeout

  1. I think it also worth updating the README with the latest features you've added.

I added all the nee Entites there and also mentioned the Events part. Or did u mean smth else?

I think just how to see the logs is missing

@AzonInc
Copy link
Collaborator Author

AzonInc commented Oct 25, 2024

I didn't save the logs so I can't tell for now. I follow and update.

Okay lets see what happens.

I think just how to see the logs is missing

Ahh okay uhm the logs are just sent to homeassistant as events. I thought thats covering the logs already.
I'm not sure what more to say about it. Maybe an explanation how to go to the developer tools page.

AFAIK ESPHome is asynchronous, and to achieve this behavior you have to use set_timeout

I will check a few options to see if they solve the delay issue.

@AzonInc
Copy link
Collaborator Author

AzonInc commented Oct 25, 2024

After using the previous PR, I got some instability (disconnections) for long periods (an hour+). I'll retry now with this new version and update.

Do you mean it disconnected for an hour or after a long period?

@uriyacovy
Copy link
Owner

After using the previous PR, I got some instability (disconnections) for long periods (an hour+). I'll retry now with this new version and update.

Do you mean it disconnected for an hour or after a long period?

Yes.
It didn't discounted since, but anyway I think we should not use long delays.

README.md Outdated Show resolved Hide resolved
@AzonInc
Copy link
Collaborator Author

AzonInc commented Oct 25, 2024

I thought about setting a timeout period in the loop and executing it when the time is up. For auth data and event logs.

Also maybe change the way data is requested and not requesting on every status change but independently.

@uriyacovy
Copy link
Owner

I thought about setting a timeout period in the loop and executing it when the time is up. For auth data and event logs.

Why not to use set_timeout instead?

Also maybe change the way data is requested and not requesting on every status change but independently.

You can do that, but just make sure that the status is updated whenever it is changed from the app/manually.

@AzonInc
Copy link
Collaborator Author

AzonInc commented Oct 25, 2024

Yea I just mean the config, advanced config and auth data + event logs because that's time consuming to request all of that after each other.

I never used set_timeout, how does it work?

@uriyacovy
Copy link
Owner

I don't have much knowledge about set_timeout, but in the discord there's some explanation:
https://discord.com/channels/429907082951524364/1234784073071722567/1234784075823190027
https://discord.com/channels/429907082951524364/1204163134064168970/1204163136144408627

@AzonInc
Copy link
Collaborator Author

AzonInc commented Oct 26, 2024

Do you think it's really necessary to request the auth data everytime or is every second time fine as well?
Because now I'm requesting auth data and then logs wehenever there is a change. However auth data doesn't change as frequently as logs. So there could be smth like a bool (initially true) which is toggled on every update and whenever it is true auth data will be requested.

@AzonInc AzonInc mentioned this pull request Oct 26, 2024
Copy link
Owner

@uriyacovy uriyacovy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @AzonInc!
Looks good. I also added you a collaborator (anyway, a large portion of the code is yours now :) )

@uriyacovy uriyacovy merged commit f198fa4 into uriyacovy:main Oct 28, 2024
1 check passed
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.

2 participants