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 support of attribute tree when available. #995

Closed
wants to merge 2 commits into from
Closed

Add support of attribute tree when available. #995

wants to merge 2 commits into from

Conversation

onlinux
Copy link
Contributor

@onlinux onlinux commented Aug 9, 2023

When devices have attributes defined as attr1.attr2.attr3 etc... you can define the attribute as attr1.attr2.attr3 etc

Ex: for RPI monitor :

type: custom:mini-graph-card
entities:

  • entity: sensor.rpi_raspi64_rpi_monitor_raspi64
    attribute: cpu.load_1min_prcnt
    name: Load Raspi64
  • entity: sensor.rpi_monitor_raspi65
    attribute: cpu.load_1min_prcnt
    name: Load Raspi65
    unit: '%'

This will prevent the need to create numerous virtual sensors for each sub-attribute you wish to access.

@onlinux
Copy link
Contributor Author

onlinux commented Aug 9, 2023

screenshot

@akloeckner
Copy link
Collaborator

akloeckner commented Aug 9, 2023

Thanks for the suggestion! I like the feature and it seems to be a quite local change.

However, I feel, there is still a bit of redundancy in the code that should be reduced:

  • The logic is spelled out two times. I'd suggest a function for it then.
  • A regular attribute is actually an edge case of your new logic, so the else branch should be avoided.
  • I'm not sure: is this double-check for an object needed? Why not simply check typeof?
  • The logic also feels a bit "literal": there are many return statements and if blocks that could be written in a more compact form.

How about adding a function like this and then just re-use it:

  function getObjectAttr (obj, path) {
      let res = obj;

      path.split('.').forEach(function (key) {
         res = typeof res === 'object' && res ? res[key] : undefined;
      });

      return res;
   }

We might have to adjust the style to the mini-graph-card habits a bit.

Also, there is some housekeeping to be done:

  • The feature should be documented. A short hint in the options' table should be enough.
  • The screenshot and the changes to package-lock.json can probably be ignored. (My setup also changes the lock file all the time, but you don't use any new dependency.)
  • Your PR should target the dev branch.

What do you think?

@ildar170975
Copy link
Collaborator

ildar170975 commented Aug 9, 2023

This functionality is very useful.
Are these usecases possible?

  1. Attribute is a dictionary. A sub attribute is called as “dictattr[“subattr”] - in addition to “dictattr.subattr”.
  2. Attribute is a list. An element is called is “listattr[2]”.

Can be tested with a weather entity.

@akloeckner
Copy link
Collaborator

Are these usecases possible?

I would keep it simple for a start:

  • [] notation will be more difficult to parse and does not give an advantage over . notation.
  • listattr[2] will thus likely not be possible, but listattr.2 could be added in a next step, once we have this feature in place. It will just be a matter of converting the string '2' to the number 2...

I suggest to get this merged as simple as possible first. And then extend.

@onlinux onlinux closed this by deleting the head repository Aug 10, 2023
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.

3 participants