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.

Sign up for GitHub

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

Jump to bottom

[paradoxalarm] Implement detailed partition state #14618

Merged
merged 6 commits into from
Oct 19, 2023
Merged

[paradoxalarm] Implement detailed partition state #14618

merged 6 commits into from
Oct 19, 2023

Conversation

theater
Copy link
Contributor

@theater theater commented Mar 17, 2023

Hi,

The idea in this PR is when a partition is in "special" state (armed in stay or armed in no entry), to calculate the value and show it as a separate, detailed channel. Some users want this more detailed state for certain calculations/rules.

The armed in stay is still an "armed" state. For backwards compatibility we will keep the regular calculated, more simple state channel and we will introduce a new channel, called detailed state where a calculated effort will return the particular state.
In the backend these values are represented by a certain bits set and usually if you trigger Arm in stay, the bit for Armed and for Armed in stay are set to 1.
If no particular special state is set on the system, the detailed state will act as our old, overall state.

Depends on #14557
Closes #14608

Cheers,
Konstantin

@theater theater added enhancement An enhancement or new feature for an existing add-on community approved awaiting other PR Depends on another PR labels Mar 17, 2023
@wborn wborn changed the title Implement detailed partition state [paradoxalarm] Implement detailed partition state Mar 17, 2023
@theater theater force-pushed the implemen_exact_partition_states branch from 56dfdaf to 8dbe56b Compare March 17, 2023 12:13
@theater theater removed the awaiting other PR Depends on another PR label Jul 26, 2023
@theater
Copy link
Contributor Author

theater commented Jul 27, 2023

Hi @fwolter, Thanks for reviewing and merging my other PR.
I wonder as you already reviewed the other PR and merged it if you could have a look at this one too?

I hoped that when the one is merged into main most commits will disappear but they stayed in the PR. Basically this PR is on top of PR #14557 and the changes start with commit 2249249
Is it better to squash the commits and try to rebase on top of main after the merge? Will that reduce the changed files in this PR?

Best regards,
Konstantin

@lolodomo
Copy link
Contributor

Of course, you have to rebase on current main branch, so that only new code is reviewed.

@theater theater force-pushed the implemen_exact_partition_states branch from 7ed0181 to ee5ba56 Compare July 27, 2023 15:24
@theater
Copy link
Contributor Author

theater commented Jul 27, 2023

OK. I rebased interactively and skipped the commits that were in the other PR. I guess that's the right thing to do...

@lolodomo
Copy link
Contributor

I probably missed your rebase. Your PR is now very small., I am going to review it.

@theater
Copy link
Contributor Author

theater commented Oct 15, 2023

Great. Looking forward it :)

@theater
Implement partition detailed state
3623494 
Signed-off-by: Konstantin Polihronov <polychronov@gmail.com>
@theater
Fix calculation of detailed state
591411e 
* Fix issue
* Additional logging
* Change detailed state type id and description

Signed-off-by: Konstantin Polihronov <polychronov@gmail.com>
@theater
Add possible values of the new channel in the README
1fdc04b 
Signed-off-by: Konstantin Polihronov <polychronov@gmail.com>
@theater
Aligned detailed states with the actual states from the system
a0e11a4 
Signed-off-by: Konstantin Polihronov <polychronov@gmail.com>
@theater
Added translation for the new channel and update.xml
938e32e 
Signed-off-by: Konstantin Polihronov <polychronov@gmail.com>
@theater theater force-pushed the implemen_exact_partition_states branch from ee5ba56 to 938e32e Compare October 17, 2023 15:40
@theater
Add thingTypeVersion property to partition.xml
1041c24 
Signed-off-by: Konstantin Polihronov <polychronov@gmail.com>
@theater
Copy link
Contributor Author

theater commented Oct 18, 2023

Hi, I did the requested changes by adding the property to the partition.xml.
I wonder if I should expect anything in particular in the logs, regarding this change?
Checked that overall the binding works as expected but not sure if it should log something regarding this update of the channel.
From what I understand the idea is the runtime to know that something got changed, so it forcefully updates the channels.

Had this issue before that some channels were not available in the UI when hot-replacing the addon but after a full OH restart, everything was OK. Not sure if these instructions to the framework address these issues...

lolodomo
lolodomo approved these changes Oct 18, 2023
View reviewed changes
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

Code LGTM, thank you

@lolodomo
Copy link
Contributor

lolodomo commented Oct 18, 2023 •
edited
Loading

Had this issue before that some channels were not available in the UI when hot-replacing the addon but after a full OH restart, everything was OK. Not sure if these instructions to the framework address these issues...

The new thing upgrade feature only concerns things added using Main UI, that is stored in JSON DB. These things were never updated, even after a server restart. With this new OH 4 feature, the thing is automatically upgraded when the new binding version starts (if upgrade instructions are provided by the developer).
For things defined in file, all this is useless, a simple restart was enough to get the last version of the thing.

I wonder if I should expect anything in particular in the logs, regarding this change?
Checked that overall the binding works as expected but not sure if it should log something regarding this update of the channel.
From what I understand the idea is the runtime to know that something got changed, so it forcefully updates the channels.

Yes. I am not sure something special is logged.

Your upgrade instructions look good to me but ideally it would be better that you first test that it works before I merge. Can you do that ?

Here is the scenario:

  1. With 4.1M2 or a recent 4.1 snapshot, create with Main UI a partition thing. Of course, you will not see in Main UI the new channel.
  2. Uninstall the paradoxalarm binding and install your new version of the binding.
  3. After the start of the binding, the new channel should then be present in Main UI for your existing partition thing.

@theater
Copy link
Contributor Author

theater commented Oct 19, 2023

Your upgrade instructions look good to me but ideally it would be better that you first test that it works before I merge. Can you do that ?

Yep. Thanks for the instructions. I did that. It works 😄

@lolodomo
Copy link
Contributor

Yep. Thanks for the instructions. I did that. It works 😄

Excellent

@lolodomo lolodomo merged commit 18ae9d4 into openhab:main Oct 19, 2023
2 checks passed
@lolodomo lolodomo added this to the 4.1 milestone Oct 19, 2023
@theater theater mentioned this pull request Oct 20, 2023
Merged
querdenker2k pushed a commit to querdenker2k/openhab-addons that referenced this pull request Oct 21, 2023
@theater @querdenker2k
[paradoxalarm] Implement detailed partition state (openhab#14618)
1d78b88 
* Implement partition detailed state

---------

Signed-off-by: Konstantin Polihronov <polychronov@gmail.com>
querdenker2k pushed a commit to querdenker2k/openhab-addons that referenced this pull request Oct 29, 2023
@theater @querdenker2k
[paradoxalarm] Implement detailed partition state (openhab#14618)
76bcf00 
* Implement partition detailed state

---------

Signed-off-by: Konstantin Polihronov <polychronov@gmail.com>
Signed-off-by: querdenker2k <querdenker2k@gmx.de>
@theater theater deleted the implemen_exact_partition_states branch November 7, 2023 14:40
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
@theater @austvik
[paradoxalarm] Implement detailed partition state (openhab#14618)
c884b65 
* Implement partition detailed state

---------

Signed-off-by: Konstantin Polihronov <polychronov@gmail.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lolodomo lolodomo lolodomo approved these changes

Assignees
No one assigned
Labels
community approved enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Milestone
4.1
Development

Successfully merging this pull request may close these issues.

[paradoxalarm] Add access to the different Alarm States (Armed, Armed in Stay, Was Is in Alarm)
2 participants
@theater @lolodomo