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

Add a persistence configuration page #1917

Merged
merged 11 commits into from
Jun 28, 2023
Merged

Conversation

florian-h05
Copy link
Contributor

@florian-h05 florian-h05 commented Jun 6, 2023

Closes #1463.
Refs openhab/openhab-core#2871.
Refs openhab/openhab-core#3642.

It is accessible from the add-on settings page and has both a design and a code tab.

The design tab allows to set persistence strategies for Items, define cron strategies and set the default strategies. It does not duplicate names for (cron) persistence strategies and filters as well as configs for the same set of Items.
All four filters provided by openHAB core (treshold, time, equals/not equals, include/exclude) can be configured.
When the user removes a cron strategy or a filter, it is automatically removed from all configs so that there is no API failure (400 Bad Request).

No code completion is not provided, but required attributes for filters are automatically set on save to avoid API failure (500 Internal Server Error).

A few words about order and sorting:

  • openHAB Core seems to sort the cron strategies.
  • Configurations itself are unsorted, they could be sorted alphabetically by the UI.
  • Items of configuration are sorted by their type (groups before normal Items) as well as alphabetically.

@florian-h05
Copy link
Contributor Author

florian-h05 commented Jun 6, 2023

To Do

  • Refactor edit, save and delete methods to de-duplicate code
  • Add debug logging and clean-up existing logging
  • Improve handling of not editable config (disable popups etc.)
  • Enable configuration of filters
Screenshots

The addon settings page looks for persistence addons like this:
image

The design tab:
image

Popup to configure Items:
image

Popup to configure cron strategies:
image

Popups to configure different filters:
image
image
image
image

The code tab:
image

@florian-h05 florian-h05 force-pushed the persistence branch 2 times, most recently from c9f6cab to 7890b94 Compare June 6, 2023 18:08
@relativeci
Copy link

relativeci bot commented Jun 6, 2023

Job #1045: Bundle Size — 15.84MiB (~+0.01%).

1b85786(current) vs 6d1e8d6 main#1044(baseline)

⚠️ Bundle contains 16 duplicate packages

Metrics (1 change)
                 Current
Job #1045
     Baseline
Job #1044
Initial JS 1.74MiB 1.74MiB
Initial CSS 608.89KiB 608.89KiB
Cache Invalidation 93.98% 93.95%
Chunks 219 219
Assets 689 689
Modules 1728 1728
Duplicate Modules 88 88
Duplicate Code 1.92% 1.92%
Packages 138 138
Duplicate Packages 15 15
Total size by type (2 changes)
                 Current
Job #1045
     Baseline
Job #1044
CSS 859.34KiB 859.34KiB
Fonts 526.1KiB 526.1KiB
HTML 1.23KiB 1.23KiB
IMG 140.74KiB 140.74KiB
JS 9.3MiB (~+0.01%) 9.3MiB
Media 295.6KiB 295.6KiB
Other 4.75MiB (~-0.01%) 4.75MiB

View job #1045 reportView main branch activity

@florian-h05 florian-h05 marked this pull request as ready for review June 6, 2023 18:36
@florian-h05 florian-h05 requested a review from a team as a code owner June 6, 2023 18:36
@florian-h05 florian-h05 added enhancement New feature or request main ui Main UI labels Jun 6, 2023
@florian-h05 florian-h05 added this to the 4.0 milestone Jun 6, 2023
@J-N-K
Copy link
Member

J-N-K commented Jun 6, 2023

Looks great. Do you think we can add configurations for filters, too (without using the code tab?)

@florian-h05
Copy link
Contributor Author

florian-h05 commented Jun 6, 2023

It should be possible, I will have a look tomorrow or in the next days.

It would be great if you could provide me real examples what the REST API returns for the filters.
Swagger helps, but does not tell everything ;-) and I haven't found documentation for the filters.

@florian-h05 florian-h05 marked this pull request as draft June 7, 2023 06:22
@J-N-K
Copy link
Member

J-N-K commented Jun 7, 2023

// persistence strategies have a name and definition and are referred to in the "Items" section
Strategies {
  everyHour : "0 0 * * * ?"
  everyDay  : "0 0 0 * * ?"
}

Filters {
  fivepercent : > % 5
  tenMilliAmps : > 10 "mA"
  atMostOnceAMinute : T 1 m
  isValid : = "first","second","third"
  notValid : ! "UNDEF","NULL"
}

/*
 * Each line in this section defines for which Item(s) which strategy(ies) should be applied.
 * You can list single items, use "*" for all items or "groupitem*" for all members of a group
 * Item (excl. the group Item itself).
 */
Items {
        // persist the Item state of Heating_Mode and Notifications_Active on every change and restore them from the db at startup
        Heating_Mode, Notifications_Active: strategy = everyChange, restoreOnStartup filter = notValid

        // additionally, persist all temperature and weather values every hour
        Temperature*, Weather* : strategy = everyHour filter = fivepercent
}
{
  "serviceId": "influxdb",
  "configs": [
    {
      "items": [
        "Heating_Mode",
        "Notifications_Active"
      ],
      "strategies": [
        "everyChange",
        "restoreOnStartup"
      ],
      "filters": [
        "notValid"
      ]
    },
    {
      "items": [
        "Temperature*",
        "Weather*"
      ],
      "strategies": [
        "everyHour"
      ],
      "filters": [
        "fivepercent"
      ]
    }
  ],
  "defaults": [],
  "cronStrategies": [
    {
      "name": "everyHour",
      "cronExpression": "0 0 * * * ?"
    },
    {
      "name": "everyDay",
      "cronExpression": "0 0 0 * * ?"
    }
  ],
  "thresholdFilters": [
    {
      "name": "fivepercent",
      "value": 5,
      "relative": true
    },
    {
      "name": "tenMilliAmps",
      "value": 10,
      "unit": "mA",
      "relative": false
    }
  ],
  "timeFilters": [
    {
      "name": "atMostOnceAMinute",
      "value": 1,
      "unit": "m"
    }
  ],
  "equalsFilters": [
    {
      "name": "isValid",
      "values": [
        "first",
        "second",
        "third"
      ],
      "inverted": false
    },
    {
      "name": "notValid",
      "values": [
        "UNDEF",
        "NULL"
      ],
      "inverted": true
    }
  ],
  "editable": false
}

@florian-h05
Copy link
Contributor Author

florian-h05 commented Jun 7, 2023

Thanks.

@J-N-K
Out of interest: Would there be any way that openHAB core provides config descriptions for the filters?
That would allow filter configuration from the UI without having to hard-code the filters into MainUI.

And your code snippet is ready for your PR openhab/openhab-core#3642, right?

@hmerk
Copy link

hmerk commented Jun 23, 2023

Any Progress here @florian-h05 ??

@florian-h05
Copy link
Contributor Author

Unfortunately not yet, I was really busy and my free time went into openhab-js.

I will try to finish it this weekend.

It is accessible from the add-on settings page and has both a design and a code tab.

The design tab allows to set persistence strategies for Items, define cron strategies and set the default strategies.
It does not duplicate names for (cron) persistence strategies and duplicate configs for the same set of Items.

The code tab also allows to also specify threshold and time filters and needs minor adjustment once openhab/openhab-core#3642 is merged code completion is not provided.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
This adds a complete filter configuration from the UI together with
support through the YAML code tab.
Filter configuration provides input validation and automatically adds
required parameters if they are not set.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
- Fix config/cron strategy already existing checks.
- Remove cron strategies and filters from configs when they are removed to avoid 400 Bad Request.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05
Copy link
Contributor Author

@J-N-K @hmerk I am finished now, all filters can be configured by UI 🎉
I updated the screenshots in #1917 (comment), you can open them under the screenshots menu.

@florian-h05 florian-h05 marked this pull request as ready for review June 23, 2023 22:47
predefinedStrategies: ['everyChange', 'everyUpdate', 'restoreOnStartup'],
filterTypes: [
{
name: 'thresholdFilters',
Copy link
Member

Choose a reason for hiding this comment

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

maybe thresholdFilter? It's the description of a single filter. Also below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't work, because the filter type name and label are used by the UI to render the sections and determine where to read from and where to store filters. And the property on the persistence is named thresholdFilters.

I have implemented the UI part for filters in a way, that once additional filters are added you only need to update the filterTypes definition and the code tab.

Co-authored-by: J-N-K <github@klug.nrw>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@stefan-hoehn
Copy link
Contributor

@hmerk I am currently testing this and we found a few minor quirks that Florian already fixed. So, almost done and looks pretty good to me. Probably can be merged by tonight.

@florian-h05 florian-h05 force-pushed the persistence branch 2 times, most recently from eefdff3 to 06e7127 Compare June 28, 2023 09:13
- Display a message when no filters are available.
- Hide the delete button in create mode.
- Pre-select everyChange as persistence strategy for new configs.
- Make all code only use the filterTypes definitions and don't rely on the existence of any filter type's array.
- Allow numbers and underslash for cron strategy & filter names.
- Validate that lower bound is less than upper bound for includeFilters.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@stefan-hoehn
Copy link
Contributor

@florian-h05 I thoroughly tested it including the latest changes - thx 4 adding my request on the validation and allowing names with _ and numbers <3

And it looks good to me from a functional perspective.

@florian-h05
Copy link
Contributor Author

@ghys This is finally ready for review! Let's get this into 4.0 🥳

Copy link
Member

@ghys ghys left a comment

Choose a reason for hiding this comment

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

Probably can be merged by tonight.

What Stefan wants, Stefan gets I suppose 😉
Jokes aside I have no quirks with this code and I think it's an important feature and therefore should be merged as early as possible so that it could be "battle-tested" before the release.

@ghys ghys merged commit 376f036 into openhab:main Jun 28, 2023
@florian-h05 florian-h05 deleted the persistence branch June 28, 2023 20:34
@hmerk
Copy link

hmerk commented Jun 29, 2023

@florian-h05
Thanks, great job.
But I have to admit, that I sruggled a bit to find the config. It was not really intuitive that the blue healine "Persistence Cinfiguration" is a link to the edit menu....

@florian-h05
Copy link
Contributor Author

florian-h05 commented Jun 29, 2023

It was not really intuitive that the blue healine "Persistence Cinfiguration" is a link to the edit menu....

I'm very open for suggestions, I am not super happy with the UX to get there.
Please also have a look at #1935 where the redesign of the settings page to allow fast access to addon settings is discussed.

@stefan-hoehn
Copy link
Contributor

Just to add on that: the documentation for the filters has been nicely documented by @J-N-K here: openhab/openhab-docs#2088

ghys pushed a commit that referenced this pull request Jul 18, 2023
Follow-up for #1917.

- Display enabled filters in config footer.
- Display filter configuration in filter footer.
- Time filter: Define options for time unit.
- Add a button to open the documentation.
- Fixes an issue, where Filter creation fails due to missing filter type
arrays.

---------

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request main ui Main UI
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Persistence configuration via UI
5 participants