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

Use of units in tag:feed names and "Undefined" units. #32

Closed
pb66 opened this issue Sep 30, 2018 · 6 comments · Fixed by #33
Closed

Use of units in tag:feed names and "Undefined" units. #32

pb66 opened this issue Sep 30, 2018 · 6 comments · Fixed by #33

Comments

@pb66
Copy link
Contributor

pb66 commented Sep 30, 2018

Just updated emoncms and graphs module on my live server and now have dozens of account each with dozens of dashboards and graphs etc, all now displaying an ugly "undefined" after every graph legend entry and in multiple places in the graph tooltips.

image

There are 2 issues here.

First and foremost is the over use of the units field when displaying feed names. A unit is part of the value not the value name. I previously mentioned this in the "added feed unit to tooltip and legend #31" PR comments.

This is an issue even if the unit is defined, for example we have key names used for the apps module that could result in a graph legend like so

emontx: solar_kwh kWh
emontx: export_kwh kWh
emontx: use_kwh kWh

Secondly, it shouldn't display "undefined" even in the correct locations, for this feature of displaying units (when defined) to be backwards compatible it must not display or just display an empty string if the unit is undefined as there are too many pre-existing feeds without units defined.

I am using latest emoncms/master and graph/master at time of writing. db updated and browser cache cleared.

@pb66 pb66 changed the title Use of units in tag and feed names and "Undefined" units. Use of units in tag:feed names and "Undefined" units. Sep 30, 2018
@Paul-Reed
Copy link
Member

Very similar to emoncms/emoncms#1000 which I reported a few weeks ago.

@pb66
Copy link
Contributor Author

pb66 commented Oct 2, 2018

Reopening this as the primary issue isn't addressed.

The unit shouldn't be tagged on the end of the feed name in the graph legend, if it must be included it should be in brackets as it is not part of the name.

Likewise in the tool-tip, the unit doesn't need to appear twice in the same tool-tip and the unit should be shown with the value rather than the name.

@pb66 pb66 reopened this Oct 2, 2018
@djh42
Copy link

djh42 commented Feb 5, 2019

I agree with what @pb66 says. I also note that the units are displayed wrongly if the feed's Scale is changed in the Options list. For example, if a feed in W is given a scale of 0.001 then the display should be labelled in in kW. Or a kWh feed with a scale of 1000 becomes a Wh display.

I appreciate that this may be complex to implement but I suggest that at a minimum the graph should not display incorrect units. So if the scale is not unity, remove the units from display.

@pb66
Copy link
Contributor Author

pb66 commented Feb 26, 2019

Recent double unit example from the forum (see https://community.openenergymonitor.org/t/beginner-question-about-correct-configuration-in-emoncms/10210?u=pb66)

image

(not sure why the formatting is on 2 lines, but my intended point here was the doubling up of units because the user has chosen to include them in the name)

@pb66
Copy link
Contributor Author

pb66 commented Feb 26, 2019

Just to throw a passing thought out there for consideration, what about adding a unit drop down to the options tab view for displaying in the legend? It could auto pre-select to the feeds set unit, but can be overridden by the user selecting a more appropriate unit from the drop down if scaling is applied and/or left blank if "None" is selected. The override value can be saved as part of the graph as per the scale etc.

@TrystanLea
Copy link
Member

I agree, I've removed the unit from the legend c5f686d

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 a pull request may close this issue.

4 participants