[netatmo] Adding Webhook event support for Doorbell #12972
Conversation
Signed-off-by: clinique <gael@lhopital.org>
Signed-off-by: clinique <gael@lhopital.org>
|
Maybe you can build a jar so that @HaKuNaCH can test and confirm it is working well with your change? |
|
@clinique - FYI, please use "Resolves #xxxxx" or "Fixes #xxxxx" next time to have issue automatically linked (and closed on merge). "Solves Issue #xxxxx" or "Solves #xxxxx" will not work. |
|
Edit: 20.06.2022 / 18:25 CEST |
|
Apparently event type RTC is missing. |
|
Does anyone has a clue of what rtc means ? I can add it but would prefer to get this precious information |
|
Now I had more time to test. These are (all) the different events from the doorbell I've seen today. Person detected: Doorbell button pressed: Incoming call, comes always after "push_type":"NDB-rtc": Missed call, when not accepted the incoming call via app: |
...ava/org/openhab/binding/netatmo/internal/handler/capability/HomeSecurityThingCapability.java
Show resolved
Hide resolved
|
c in RTC could be for call. |
According to dev.netatmo.com (where this event is not related to doorbell btw) it seems to just be "Button pressed" :) |
Enhancing NAPushType deserialization Signed-off-by: clinique <gael@lhopital.org>
I have tested the for message types you were kind enough to list. Everything seems fine. I have delivered you another test version on the shared folder. |
Signed-off-by: clinique <gael@lhopital.org>
|
Thanks for the new version. Movement detected Channel-items updated: Until the next event, the following items get updated: Movement detected / incoming call / unanswered Items updated: Incoming call: Items updated: Webhook content in openhab.log Items updated: Webhook content in openhab.log Items updated: Incoming call / answered (webhook from netatmo does not send "answered-call") Items updated: Webhook content in openhab.log Items updated: afterwards without any action items updated: |
@clinique : can you confirm it is expected? @jlaur : you are welcome to finish this review in time for 3.3, I am sorry, I will not have much time before next weekend. |
|
I confirm this is not as it should be : empty values should go to "NULL". Will post an update on this soon. Regarding automatic refresh, it can compete with delivery of web hook events because it will automatically request last available event for the device (that should be the same that last webhook triggered I guess - but not predictable). Would this mean that when a webhook is set, we should ignore events coming from automatic refresh ? I think this is too late in regard of 3.3 to make such a modification. |
|
@HaKuNaCH : updated version available in the folder. |
Signed-off-by: clinique <gael@lhopital.org>
|
@lolodomo @clinique Currently I use the "HTTPListener Binding" for the netatmo webhook integration. Finally, I can test today evening because I have no access to my installation atm. |
|
@clinique Channels updated with NULL: Channels with emtpy value: But as mentioned in my last comment, the binding should not overwrite existing values of all these channels with NULL. |
@clinique |
|
To be complete ... This happens on restart of OH or reenable the netatmo account thing. |
The question is to always get the snapshot associated to the last event. If the last event has no snapshot, you must not keep the snapshot of a previous event. |
|
But every event has a snapshot except "push_type":"NDB-rtc", but after that event there is always an "push_type":"NDB-incoming_call" afterwards within a few seconds. |
|
Why the polling is not returning the last event pushed through webhook? IMHO, we should have the same data as we get when clicking on the last event in the netatmo app. That event is not providing all data fields, it depends on event type. With the welcome camera, not all events have a snapshot, for example when camera monitoring is enabled or disabled. In this case, we do not want to keep the snapshot of an outdated event that could one day before. We have to find a general behaviour that will work for any Netatmo device, and not only something suitable for doorbell. The combination of polling events and webhook events is a good question but I was expecting that the last event to be the same in the two cases. |
|
3.3 RC1 will be published in less than 2 days. We have to decide quickly what to do. I have the feeling it is better to have something working for webhook than nothing at all. If the current implementation does not satisfy everyone it could be discussed and enhanced after 3.3 is released. |
Agree that this has to be thought. I would not agree on a partial update on the kind of event. I would rather prefer implement something like "if device expects webhook events then it should not be updated by pulled events" after the initial one. |
|
Ok so this PR will be delayed after 3 3 is released. |
|
I guess the webhook part ist ok, because I tested now the call on the API directly. /homedata -> if required, I can post an example |
|
Agree with @HaKuNaCH , I think the way webhooks are handled in here is correct but this raises the need for an enhancement in the regular pull part. |
|
So you all agree that this PR should be merged in its current state (after a code review)? |
|
Yes |
| result.add(getCameraId()); | ||
| addNotBlank(result, homeId); | ||
| addNotBlank(result, deviceId); | ||
| addNotBlank(result, getCameraId()); |
There was a problem hiding this comment.
Is it expected to add camera ID twice?
There was a problem hiding this comment.
Yes, it is. Some values may be empty. In some cases (e.g. smoke detector), the cameraId will be blank but the deviceId will be filled. Redundant entries will be ignored.
There was a problem hiding this comment.
I am not fully convinced, there is certainly something I do not understand...
As you are apparently convinced you have to call this twice, let's go, we have no more time to debate.
Also a YES from my side. Should I open a new bug report for the polling issue with the Doorbell? |
Yes, please, do. |
done: #13002 |
* Adding Webhook event support for Doorbell * Adding doorbell rtc. * Enhancing NAPushType deserialization * Setting empty fields to NULL Signed-off-by: clinique <gael@lhopital.org>
* Adding Webhook event support for Doorbell * Adding doorbell rtc. * Enhancing NAPushType deserialization * Setting empty fields to NULL Signed-off-by: clinique <gael@lhopital.org>
* Adding Webhook event support for Doorbell * Adding doorbell rtc. * Enhancing NAPushType deserialization * Setting empty fields to NULL Signed-off-by: clinique <gael@lhopital.org> Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
* Adding Webhook event support for Doorbell * Adding doorbell rtc. * Enhancing NAPushType deserialization * Setting empty fields to NULL Signed-off-by: clinique <gael@lhopital.org>
* Adding Webhook event support for Doorbell * Adding doorbell rtc. * Enhancing NAPushType deserialization * Setting empty fields to NULL Signed-off-by: clinique <gael@lhopital.org>
Resolves #12952
Signed-off-by: clinique gael@lhopital.org