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

Unpacking RESTful sensor JSON results into attributes. #10753

Merged
merged 32 commits into from
Dec 3, 2017
Merged

Unpacking RESTful sensor JSON results into attributes. #10753

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
dbaa626
Added support for extracting JSON attributes from RESTful values
nickovs May 6, 2017
fb19f87
Added support for extracting JSON attributes from RESTful values
nickovs May 6, 2017
0cb9def
Merge branch 'rest-json-attrs' of https://github.com/nickovs/home-ass…
nickovs May 6, 2017
e08f649
Added requirement that RESTful JSON results used as attributes must be
nickovs May 6, 2017
de07ae5
Expanded test coverage to test REFTful JSON attributes with and
nickovs May 6, 2017
227a5ad
Added support for extracting JSON attributes from RESTful values
nickovs May 6, 2017
769c7ff
Added requirement that RESTful JSON results used as attributes must be
nickovs May 6, 2017
0f25f6d
Expanded test coverage to test REFTful JSON attributes with and
nickovs May 6, 2017
a22a6d4
Merge branch 'rest-json-attrs' of github.com:nickovs/home-assistant i…
nickovs May 7, 2017
6bd57cc
sensor.envirophat: add missing requirement (#7451)
imrehg May 5, 2017
d635e26
PyPI Openzwave (#7415)
JshWright May 5, 2017
44cc762
Add datadog component (#7158)
nunofgs May 5, 2017
67e8f52
Fix object type for default KNX port
JshWright May 5, 2017
d23ccce
Added support for extracting JSON attributes from RESTful values
nickovs May 6, 2017
f5bbf6b
Added requirement that RESTful JSON results used as attributes must be
nickovs May 6, 2017
a281132
Expanded test coverage to test REFTful JSON attributes with and
nickovs May 6, 2017
dda455d
Added support for extracting JSON attributes from RESTful values
nickovs May 6, 2017
3a99f4e
Added requirement that RESTful JSON results used as attributes must be
nickovs May 6, 2017
22bc0fc
Expanded test coverage to test REFTful JSON attributes with and
nickovs May 6, 2017
d19a81b
Added support for extracting JSON attributes from RESTful values
nickovs May 6, 2017
4ef54a1
Added requirement that RESTful JSON results used as attributes must be
nickovs May 6, 2017
0b9711c
Expanded test coverage to test REFTful JSON attributes with and
nickovs May 6, 2017
8c5548e
Merge branch 'rest-json-attrs' of github.com:nickovs/home-assistant i…
nickovs Nov 20, 2017
0795823
Brought up to date with most recent upstream master
nickovs Nov 20, 2017
19eab69
Fixed breaks cause by manual upstream merge.
nickovs Nov 22, 2017
9dd2a5a
Added one extra blank line to make PyLint happy.
nickovs Nov 22, 2017
2dbcff5
Switched json_attributes to be a list of keys rather than a boolean.
nickovs Nov 28, 2017
1d36270
Added an explicit default value to the json_attributes config entry.
nickovs Dec 2, 2017
02158b0
Removed self.update() from __init__() body.
nickovs Dec 2, 2017
a537b45
Merge branch 'rest-json-attrs' of github.com:nickovs/home-assistant i…
nickovs Dec 2, 2017
3a9650c
Expended unit tests for error cases of json_attributes processing.
nickovs Dec 3, 2017
0bf5ea3
Align quotes
fabaff Dec 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Switched json_attributes to be a list of keys rather than a boolean. 
The value of json_attributes can now be either a comma sepaated list
of key names or a YAML list of key names. Only matching keys in a
retuned JSON dictionary will be mapped to sensor attributes.

Updated test cases to handle json_attributes being a list.

Also fixed two minor issues arrising from manual merge with 0.58 master.
  • Loading branch information
@nickovs
nickovs committed Nov 28, 2017
commit 2dbcff5259e0703255e46e7ed3e3b6f962d875e0
13 changes: 7 additions & 6 deletions homeassistant/components/sensor/rest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
vol.Optional(CONF_USERNAME): cv.string,
vol.Optional(CONF_VALUE_TEMPLATE): cv.template,
vol.Optional(CONF_VERIFY_SSL, default=DEFAULT_VERIFY_SSL): cv.boolean,
vol.Optional(CONF_JSON_ATTRS): cv.boolean,
vol.Optional(CONF_JSON_ATTRS): cv.ensure_list_csv,
Copy link
Member

Choose a reason for hiding this comment

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

According to your docs the default is an empty list (-> default=[]). To be in sync with other platform we should only accept a list and not a comma-separated string ->vol.Optional(CONF_JSON_ATTRS): vol.All(cv.ensure_list, [cv.string]),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added the default (although the existing code treats the a value of None the same as an empty list).

Regarding the use of cv.ensure_list_csv, is this function deprecated? The function seems to be there explicitly to allow users to enter lists either as a YAML list or as a CVS list, which is very helpful to users and allows them to choose the representation that they find most readable and compact. The function gets used by calendar/todoist.py, insteon_plm, sensor/mvglive.py and switch/rpi_rf.py and serves a very useful purpose. If the intention is to do way with this useful capability then I can remove it from here but if the function is going to stay around is there really a compelling reason not to use it?

Copy link
Member

Choose a reason for hiding this comment

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

No, it's not deprecated and was introduced for the customizing part. I was wrong...it works for all possible ways:

    json_attributes:
      - "time"
      - "date"
      - "milliseconds_since_epoch"
    json_attributes: ["time", "date", "milliseconds_since_epoch"]
    json_attributes: time, date, milliseconds_since_epoch
    json_attributes: time,date,milliseconds_since_epoch
    json_attributes: "time", "date", "milliseconds_since_epoch"

But I still would prefer if we don't promote a, b, c because it will lead to confusion for users which are not that familiar with YAML and are looking at other components/platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great. The documentation pull request that I submitted uses option d for the example but I'm happy to document it with whichever form you think should be promoted.

})


Expand Down Expand Up @@ -73,7 +73,7 @@ def setup_platform(hass, config, add_devices, discovery_info=None):
rest.update()

add_devices([RestSensor(hass, rest, name, unit,
value_template, json_attrs)])
value_template, json_attrs)], True)


class RestSensor(Entity):
Expand All @@ -90,7 +90,6 @@ def __init__(self, hass, rest, name,
self._value_template = value_template
self._json_attrs = json_attrs
self._attributes = None
self.update()

@property
def name(self):
Expand Down Expand Up @@ -120,9 +119,11 @@ def update(self):
if self._json_attrs:
self._attributes = None
try:
attrs = json.loads(value)
if isinstance(attrs, dict):
self._attributes = json.loads(value)
json_dict = json.loads(value)
if isinstance(json_dict, dict):
attrs = {k: json_dict[k] for k in self._json_attrs
if k in json_dict}
self._attributes = attrs
else:
_LOGGER.warning('JSON result was not a dictionary')
except ValueError:
Expand Down
12 changes: 6 additions & 6 deletions tests/components/sensor/test_rest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def setUp(self):

self.sensor = rest.RestSensor(self.hass, self.rest, self.name,
self.unit_of_measurement,
self.value_template, False)
self.value_template, [])

def tearDown(self):
"""Stop everything that was started."""
Expand Down Expand Up @@ -182,30 +182,30 @@ def test_update_with_no_template(self):
side_effect=self.update_side_effect(
'plain_state'))
self.sensor = rest.RestSensor(self.hass, self.rest, self.name,
self.unit_of_measurement, None, False)
self.unit_of_measurement, None, [])
self.sensor.update()
self.assertEqual('plain_state', self.sensor.state)
self.assertTrue(self.sensor.available)

def test_update_with_josn_attrs(self):
def test_update_with_json_attrs(self):
"""Test attributes get extracted from a JSON result."""
self.rest.update = Mock('rest.RestData.update',
side_effect=self.update_side_effect(
'{ "key": "some_json_value" }'))
self.sensor = rest.RestSensor(self.hass, self.rest, self.name,
self.unit_of_measurement, None, True)
self.unit_of_measurement, None, ['key'])
self.sensor.update()
self.assertEqual('some_json_value',
self.sensor.device_state_attributes['key'])

def test_update_with_josn_attrs_and_template(self):
def test_update_with_json_attrs_and_template(self):
"""Test attributes get extracted from a JSON result."""
self.rest.update = Mock('rest.RestData.update',
side_effect=self.update_side_effect(
'{ "key": "json_state_updated_value" }'))
self.sensor = rest.RestSensor(self.hass, self.rest, self.name,
self.unit_of_measurement,
self.value_template, True)
self.value_template, ['key'])
self.sensor.update()

self.assertEqual('json_state_updated_value', self.sensor.state)
Expand Down