Skip to content
Snippets Groups Projects
Commit 995758b8 authored by Jc2k's avatar Jc2k Committed by Martin Hjelmare
Browse files

Add more HomeKit controller tests (#20515)

* homekit_controller tests: automatically find entity ids in tests

Some entities use dynamic ids because of the nature of the test fakes it is
hard to predict the name of the entity that will be created. This inspects the
EntityComponent of the domain to find the freshly created entity.

* homekit_controller: Tests can now define their own Service models.

All existing tests use models as defined upstream. But upstream only defines a
few service models. This adds a generic model helper for creating test
service/characteristic models.

* homekit_controller: Add cover tests

* homekit_controller: Add lock tests

* homekit_controller: Add alarm_control_panel tests

* homekit_controller: Update light tests for color_temp.

* Revert "homekit_controller tests: automatically find entity ids in tests"

This reverts commit 506caa4c3e0814eec637145d7d6eaf2bd642e99e.

* homekit_controller: Mock entity name so entity_id is consistent.

Also remove spurious subclass overrides that are identical to parent class.

* homekit_controler: Make tests less awkward as allowed top level imports
parent f33e432c
No related branches found
No related tags found
No related merge requests found
...@@ -94,11 +94,6 @@ class HomeKitGarageDoorCover(HomeKitEntity, CoverDevice): ...@@ -94,11 +94,6 @@ class HomeKitGarageDoorCover(HomeKitEntity, CoverDevice):
self._chars['name'] = characteristic['iid'] self._chars['name'] = characteristic['iid']
self._name = characteristic['value'] self._name = characteristic['value']
@property
def name(self):
"""Return the name of the cover."""
return self._name
@property @property
def available(self): def available(self):
"""Return True if entity is available.""" """Return True if entity is available."""
...@@ -206,7 +201,7 @@ class HomeKitWindowCover(HomeKitEntity, CoverDevice): ...@@ -206,7 +201,7 @@ class HomeKitWindowCover(HomeKitEntity, CoverDevice):
self._chars['vertical-tilt.target'] = \ self._chars['vertical-tilt.target'] = \
characteristic['iid'] characteristic['iid']
elif ctype == "horizontal-tilt.target": elif ctype == "horizontal-tilt.target":
self._chars['vertical-tilt.target'] = \ self._chars['horizontal-tilt.target'] = \
characteristic['iid'] characteristic['iid']
elif ctype == "obstruction-detected": elif ctype == "obstruction-detected":
self._chars['obstruction-detected'] = characteristic['iid'] self._chars['obstruction-detected'] = characteristic['iid']
...@@ -216,11 +211,6 @@ class HomeKitWindowCover(HomeKitEntity, CoverDevice): ...@@ -216,11 +211,6 @@ class HomeKitWindowCover(HomeKitEntity, CoverDevice):
if 'value' in characteristic: if 'value' in characteristic:
self._name = characteristic['value'] self._name = characteristic['value']
@property
def name(self):
"""Return the name of the cover."""
return self._name
@property @property
def supported_features(self): def supported_features(self):
"""Flag supported features.""" """Flag supported features."""
......
...@@ -52,7 +52,7 @@ class HomeKitLight(HomeKitEntity, Light): ...@@ -52,7 +52,7 @@ class HomeKitLight(HomeKitEntity, Light):
self._features |= SUPPORT_BRIGHTNESS self._features |= SUPPORT_BRIGHTNESS
self._brightness = characteristic['value'] self._brightness = characteristic['value']
elif ctype == 'color-temperature': elif ctype == 'color-temperature':
self._chars['color_temperature'] = characteristic['iid'] self._chars['color-temperature'] = characteristic['iid']
self._features |= SUPPORT_COLOR_TEMP self._features |= SUPPORT_COLOR_TEMP
self._color_temperature = characteristic['value'] self._color_temperature = characteristic['value']
elif ctype == "hue": elif ctype == "hue":
......
...@@ -2,8 +2,13 @@ ...@@ -2,8 +2,13 @@
from datetime import timedelta from datetime import timedelta
from unittest import mock from unittest import mock
from homekit.model.services import AbstractService, ServicesTypes
from homekit.model.characteristics import (
AbstractCharacteristic, CharacteristicPermissions, CharacteristicsTypes)
from homekit.model import Accessory, get_id
from homeassistant.components.homekit_controller import ( from homeassistant.components.homekit_controller import (
DOMAIN, HOMEKIT_ACCESSORY_DISPATCH, SERVICE_HOMEKIT) DOMAIN, HOMEKIT_ACCESSORY_DISPATCH, SERVICE_HOMEKIT, HomeKitEntity)
from homeassistant.setup import async_setup_component from homeassistant.setup import async_setup_component
import homeassistant.util.dt as dt_util import homeassistant.util.dt as dt_util
from tests.common import async_fire_time_changed, fire_service_discovered from tests.common import async_fire_time_changed, fire_service_discovered
...@@ -75,9 +80,6 @@ class Helper: ...@@ -75,9 +80,6 @@ class Helper:
def __init__(self, hass, entity_id, pairing, accessory): def __init__(self, hass, entity_id, pairing, accessory):
"""Create a helper for a given accessory/entity.""" """Create a helper for a given accessory/entity."""
from homekit.model.services import ServicesTypes
from homekit.model.characteristics import CharacteristicsTypes
self.hass = hass self.hass = hass
self.entity_id = entity_id self.entity_id = entity_id
self.pairing = pairing self.pairing = pairing
...@@ -101,11 +103,39 @@ class Helper: ...@@ -101,11 +103,39 @@ class Helper:
return state return state
class FakeCharacteristic(AbstractCharacteristic):
"""
A model of a generic HomeKit characteristic.
Base is abstract and can't be instanced directly so this subclass is
needed even though it doesn't add any methods.
"""
pass
class FakeService(AbstractService):
"""A model of a generic HomeKit service."""
def __init__(self, service_name):
"""Create a fake service by its short form HAP spec name."""
char_type = ServicesTypes.get_uuid(service_name)
super().__init__(char_type, get_id())
def add_characteristic(self, name):
"""Add a characteristic to this service by name."""
full_name = 'public.hap.characteristic.' + name
char = FakeCharacteristic(get_id(), full_name, None)
char.perms = [
CharacteristicPermissions.paired_read,
CharacteristicPermissions.paired_write
]
self.characteristics.append(char)
return char
async def setup_test_component(hass, services): async def setup_test_component(hass, services):
"""Load a fake homekit accessory based on a homekit accessory model.""" """Load a fake homekit accessory based on a homekit accessory model."""
from homekit.model import Accessory
from homekit.model.services import ServicesTypes
domain = None domain = None
for service in services: for service in services:
service_name = ServicesTypes.get_short(service.type) service_name = ServicesTypes.get_short(service.type)
...@@ -138,7 +168,8 @@ async def setup_test_component(hass, services): ...@@ -138,7 +168,8 @@ async def setup_test_component(hass, services):
} }
} }
fire_service_discovered(hass, SERVICE_HOMEKIT, discovery_info) with mock.patch.object(HomeKitEntity, 'name', 'testdevice'):
await hass.async_block_till_done() fire_service_discovered(hass, SERVICE_HOMEKIT, discovery_info)
await hass.async_block_till_done()
return Helper(hass, '.'.join((domain, 'testdevice')), pairing, accessory) return Helper(hass, '.'.join((domain, 'testdevice')), pairing, accessory)
"""Basic checks for HomeKitalarm_control_panel."""
from tests.components.homekit_controller.common import (
FakeService, setup_test_component)
CURRENT_STATE = ('security-system', 'security-system-state.current')
TARGET_STATE = ('security-system', 'security-system-state.target')
def create_security_system_service():
"""Define a security-system characteristics as per page 219 of HAP spec."""
service = FakeService('public.hap.service.security-system')
cur_state = service.add_characteristic('security-system-state.current')
cur_state.value = 0
targ_state = service.add_characteristic('security-system-state.target')
targ_state.value = 0
# According to the spec, a battery-level characteristic is normally
# part of a seperate service. However as the code was written (which
# predates this test) the battery level would have to be part of the lock
# service as it is here.
targ_state = service.add_characteristic('battery-level')
targ_state.value = 50
return service
async def test_switch_change_alarm_state(hass, utcnow):
"""Test that we can turn a HomeKit alarm on and off again."""
alarm_control_panel = create_security_system_service()
helper = await setup_test_component(hass, [alarm_control_panel])
await hass.services.async_call('alarm_control_panel', 'alarm_arm_home', {
'entity_id': 'alarm_control_panel.testdevice',
}, blocking=True)
assert helper.characteristics[TARGET_STATE].value == 0
await hass.services.async_call('alarm_control_panel', 'alarm_arm_away', {
'entity_id': 'alarm_control_panel.testdevice',
}, blocking=True)
assert helper.characteristics[TARGET_STATE].value == 1
await hass.services.async_call('alarm_control_panel', 'alarm_arm_night', {
'entity_id': 'alarm_control_panel.testdevice',
}, blocking=True)
assert helper.characteristics[TARGET_STATE].value == 2
await hass.services.async_call('alarm_control_panel', 'alarm_disarm', {
'entity_id': 'alarm_control_panel.testdevice',
}, blocking=True)
assert helper.characteristics[TARGET_STATE].value == 3
async def test_switch_read_alarm_state(hass, utcnow):
"""Test that we can read the state of a HomeKit alarm accessory."""
alarm_control_panel = create_security_system_service()
helper = await setup_test_component(hass, [alarm_control_panel])
helper.characteristics[CURRENT_STATE].value = 0
state = await helper.poll_and_get_state()
assert state.state == 'armed_home'
assert state.attributes['battery_level'] == 50
helper.characteristics[CURRENT_STATE].value = 1
state = await helper.poll_and_get_state()
assert state.state == 'armed_away'
helper.characteristics[CURRENT_STATE].value = 2
state = await helper.poll_and_get_state()
assert state.state == 'armed_night'
helper.characteristics[CURRENT_STATE].value = 3
state = await helper.poll_and_get_state()
assert state.state == 'disarmed'
helper.characteristics[CURRENT_STATE].value = 4
state = await helper.poll_and_get_state()
assert state.state == 'triggered'
"""Basic checks for HomeKitalarm_control_panel."""
from tests.components.homekit_controller.common import (
FakeService, setup_test_component)
POSITION_STATE = ('window-covering', 'position.state')
POSITION_CURRENT = ('window-covering', 'position.current')
POSITION_TARGET = ('window-covering', 'position.target')
H_TILT_CURRENT = ('window-covering', 'horizontal-tilt.current')
H_TILT_TARGET = ('window-covering', 'horizontal-tilt.target')
V_TILT_CURRENT = ('window-covering', 'vertical-tilt.current')
V_TILT_TARGET = ('window-covering', 'vertical-tilt.target')
WINDOW_OBSTRUCTION = ('window-covering', 'obstruction-detected')
DOOR_CURRENT = ('garage-door-opener', 'door-state.current')
DOOR_TARGET = ('garage-door-opener', 'door-state.target')
DOOR_OBSTRUCTION = ('garage-door-opener', 'obstruction-detected')
def create_window_covering_service():
"""Define a window-covering characteristics as per page 219 of HAP spec."""
service = FakeService('public.hap.service.window-covering')
cur_state = service.add_characteristic('position.current')
cur_state.value = 0
targ_state = service.add_characteristic('position.target')
targ_state.value = 0
position_state = service.add_characteristic('position.state')
position_state.value = 0
position_hold = service.add_characteristic('position.hold')
position_hold.value = 0
obstruction = service.add_characteristic('obstruction-detected')
obstruction.value = False
name = service.add_characteristic('name')
name.value = "Window Cover 1"
return service
def create_window_covering_service_with_h_tilt():
"""Define a window-covering characteristics as per page 219 of HAP spec."""
service = create_window_covering_service()
tilt_current = service.add_characteristic('horizontal-tilt.current')
tilt_current.value = 0
tilt_target = service.add_characteristic('horizontal-tilt.target')
tilt_target.value = 0
return service
def create_window_covering_service_with_v_tilt():
"""Define a window-covering characteristics as per page 219 of HAP spec."""
service = create_window_covering_service()
tilt_current = service.add_characteristic('vertical-tilt.current')
tilt_current.value = 0
tilt_target = service.add_characteristic('vertical-tilt.target')
tilt_target.value = 0
return service
async def test_change_window_cover_state(hass, utcnow):
"""Test that we can turn a HomeKit alarm on and off again."""
window_cover = create_window_covering_service()
helper = await setup_test_component(hass, [window_cover])
await hass.services.async_call('cover', 'open_cover', {
'entity_id': helper.entity_id,
}, blocking=True)
assert helper.characteristics[POSITION_TARGET].value == 100
await hass.services.async_call('cover', 'close_cover', {
'entity_id': helper.entity_id,
}, blocking=True)
assert helper.characteristics[POSITION_TARGET].value == 0
async def test_read_window_cover_state(hass, utcnow):
"""Test that we can read the state of a HomeKit alarm accessory."""
window_cover = create_window_covering_service()
helper = await setup_test_component(hass, [window_cover])
helper.characteristics[POSITION_STATE].value = 0
state = await helper.poll_and_get_state()
assert state.state == 'opening'
helper.characteristics[POSITION_STATE].value = 1
state = await helper.poll_and_get_state()
assert state.state == 'closing'
helper.characteristics[POSITION_STATE].value = 2
state = await helper.poll_and_get_state()
assert state.state == 'closed'
helper.characteristics[WINDOW_OBSTRUCTION].value = True
state = await helper.poll_and_get_state()
assert state.attributes['obstruction-detected'] is True
async def test_read_window_cover_tilt_horizontal(hass, utcnow):
"""Test that horizontal tilt is handled correctly."""
window_cover = create_window_covering_service_with_h_tilt()
helper = await setup_test_component(hass, [window_cover])
helper.characteristics[H_TILT_CURRENT].value = 75
state = await helper.poll_and_get_state()
assert state.attributes['current_tilt_position'] == 75
async def test_read_window_cover_tilt_vertical(hass, utcnow):
"""Test that vertical tilt is handled correctly."""
window_cover = create_window_covering_service_with_v_tilt()
helper = await setup_test_component(hass, [window_cover])
helper.characteristics[V_TILT_CURRENT].value = 75
state = await helper.poll_and_get_state()
assert state.attributes['current_tilt_position'] == 75
async def test_write_window_cover_tilt_horizontal(hass, utcnow):
"""Test that horizontal tilt is written correctly."""
window_cover = create_window_covering_service_with_h_tilt()
helper = await setup_test_component(hass, [window_cover])
await hass.services.async_call('cover', 'set_cover_tilt_position', {
'entity_id': helper.entity_id,
'tilt_position': 90
}, blocking=True)
assert helper.characteristics[H_TILT_TARGET].value == 90
async def test_write_window_cover_tilt_vertical(hass, utcnow):
"""Test that vertical tilt is written correctly."""
window_cover = create_window_covering_service_with_v_tilt()
helper = await setup_test_component(hass, [window_cover])
await hass.services.async_call('cover', 'set_cover_tilt_position', {
'entity_id': helper.entity_id,
'tilt_position': 90
}, blocking=True)
assert helper.characteristics[V_TILT_TARGET].value == 90
def create_garage_door_opener_service():
"""Define a garage-door-opener chars as per page 217 of HAP spec."""
service = FakeService('public.hap.service.garage-door-opener')
cur_state = service.add_characteristic('door-state.current')
cur_state.value = 0
targ_state = service.add_characteristic('door-state.target')
targ_state.value = 0
obstruction = service.add_characteristic('obstruction-detected')
obstruction.value = False
name = service.add_characteristic('name')
name.value = "Garage Door Opener 1"
return service
async def test_change_door_state(hass, utcnow):
"""Test that we can turn open and close a HomeKit garage door."""
door = create_garage_door_opener_service()
helper = await setup_test_component(hass, [door])
await hass.services.async_call('cover', 'open_cover', {
'entity_id': helper.entity_id,
}, blocking=True)
assert helper.characteristics[DOOR_TARGET].value == 0
await hass.services.async_call('cover', 'close_cover', {
'entity_id': helper.entity_id,
}, blocking=True)
assert helper.characteristics[DOOR_TARGET].value == 1
async def test_read_door_state(hass, utcnow):
"""Test that we can read the state of a HomeKit garage door."""
door = create_garage_door_opener_service()
helper = await setup_test_component(hass, [door])
helper.characteristics[DOOR_CURRENT].value = 0
state = await helper.poll_and_get_state()
assert state.state == 'open'
helper.characteristics[DOOR_CURRENT].value = 1
state = await helper.poll_and_get_state()
assert state.state == 'closed'
helper.characteristics[DOOR_CURRENT].value = 2
state = await helper.poll_and_get_state()
assert state.state == 'opening'
helper.characteristics[DOOR_CURRENT].value = 3
state = await helper.poll_and_get_state()
assert state.state == 'closing'
helper.characteristics[DOOR_OBSTRUCTION].value = True
state = await helper.poll_and_get_state()
assert state.attributes['obstruction-detected'] is True
"""Basic checks for HomeKitSwitch.""" """Basic checks for HomeKitSwitch."""
from tests.components.homekit_controller.common import ( from tests.components.homekit_controller.common import (
setup_test_component) FakeService, setup_test_component)
LIGHT_ON = ('lightbulb', 'on')
LIGHT_BRIGHTNESS = ('lightbulb', 'brightness')
LIGHT_HUE = ('lightbulb', 'hue')
LIGHT_SATURATION = ('lightbulb', 'saturation')
LIGHT_COLOR_TEMP = ('lightbulb', 'color-temperature')
def create_lightbulb_service():
"""Define lightbulb characteristics."""
service = FakeService('public.hap.service.lightbulb')
on_char = service.add_characteristic('on')
on_char.value = 0
brightness = service.add_characteristic('brightness')
brightness.value = 0
return service
def create_lightbulb_service_with_hs():
"""Define a lightbulb service with hue + saturation."""
service = create_lightbulb_service()
hue = service.add_characteristic('hue')
hue.value = 0
saturation = service.add_characteristic('saturation')
saturation.value = 0
return service
def create_lightbulb_service_with_color_temp():
"""Define a lightbulb service with color temp."""
service = create_lightbulb_service()
color_temp = service.add_characteristic('color-temperature')
color_temp.value = 0
return service
async def test_switch_change_light_state(hass, utcnow): async def test_switch_change_light_state(hass, utcnow):
"""Test that we can turn a HomeKit light on and off again.""" """Test that we can turn a HomeKit light on and off again."""
from homekit.model.services import BHSLightBulbService bulb = create_lightbulb_service_with_hs()
helper = await setup_test_component(hass, [bulb])
helper = await setup_test_component(hass, [BHSLightBulbService()])
await hass.services.async_call('light', 'turn_on', { await hass.services.async_call('light', 'turn_on', {
'entity_id': 'light.testdevice', 'entity_id': 'light.testdevice',
'brightness': 255, 'brightness': 255,
'hs_color': [4, 5], 'hs_color': [4, 5],
}, blocking=True) }, blocking=True)
assert helper.characteristics[('lightbulb', 'on')].value == 1
assert helper.characteristics[('lightbulb', 'brightness')].value == 100 assert helper.characteristics[LIGHT_ON].value == 1
assert helper.characteristics[('lightbulb', 'hue')].value == 4 assert helper.characteristics[LIGHT_BRIGHTNESS].value == 100
assert helper.characteristics[('lightbulb', 'saturation')].value == 5 assert helper.characteristics[LIGHT_HUE].value == 4
assert helper.characteristics[LIGHT_SATURATION].value == 5
await hass.services.async_call('light', 'turn_off', { await hass.services.async_call('light', 'turn_off', {
'entity_id': 'light.testdevice', 'entity_id': 'light.testdevice',
}, blocking=True) }, blocking=True)
assert helper.characteristics[('lightbulb', 'on')].value == 0 assert helper.characteristics[LIGHT_ON].value == 0
async def test_switch_change_light_state_color_temp(hass, utcnow):
"""Test that we can turn change color_temp."""
bulb = create_lightbulb_service_with_color_temp()
helper = await setup_test_component(hass, [bulb])
await hass.services.async_call('light', 'turn_on', {
'entity_id': 'light.testdevice',
'brightness': 255,
'color_temp': 400,
}, blocking=True)
assert helper.characteristics[LIGHT_ON].value == 1
assert helper.characteristics[LIGHT_BRIGHTNESS].value == 100
assert helper.characteristics[LIGHT_COLOR_TEMP].value == 400
async def test_switch_read_light_state(hass, utcnow): async def test_switch_read_light_state(hass, utcnow):
"""Test that we can read the state of a HomeKit light accessory.""" """Test that we can read the state of a HomeKit light accessory."""
from homekit.model.services import BHSLightBulbService bulb = create_lightbulb_service_with_hs()
helper = await setup_test_component(hass, [bulb])
helper = await setup_test_component(hass, [BHSLightBulbService()])
# Initial state is that the light is off # Initial state is that the light is off
state = await helper.poll_and_get_state() state = await helper.poll_and_get_state()
assert state.state == 'off' assert state.state == 'off'
# Simulate that someone switched on the device in the real world not via HA # Simulate that someone switched on the device in the real world not via HA
helper.characteristics[('lightbulb', 'on')].set_value(True) helper.characteristics[LIGHT_ON].set_value(True)
helper.characteristics[LIGHT_BRIGHTNESS].value = 100
helper.characteristics[LIGHT_HUE].value = 4
helper.characteristics[LIGHT_SATURATION].value = 5
state = await helper.poll_and_get_state() state = await helper.poll_and_get_state()
assert state.state == 'on' assert state.state == 'on'
assert state.attributes['brightness'] == 255
assert state.attributes['hs_color'] == (4, 5)
# Simulate that device switched off in the real world not via HA # Simulate that device switched off in the real world not via HA
helper.characteristics[('lightbulb', 'on')].set_value(False) helper.characteristics[LIGHT_ON].set_value(False)
state = await helper.poll_and_get_state()
assert state.state == 'off'
async def test_switch_read_light_state_color_temp(hass, utcnow):
"""Test that we can read the color_temp of a light accessory."""
bulb = create_lightbulb_service_with_color_temp()
helper = await setup_test_component(hass, [bulb])
# Initial state is that the light is off
state = await helper.poll_and_get_state() state = await helper.poll_and_get_state()
assert state.state == 'off' assert state.state == 'off'
# Simulate that someone switched on the device in the real world not via HA
helper.characteristics[LIGHT_ON].set_value(True)
helper.characteristics[LIGHT_BRIGHTNESS].value = 100
helper.characteristics[LIGHT_COLOR_TEMP].value = 400
state = await helper.poll_and_get_state()
assert state.state == 'on'
assert state.attributes['brightness'] == 255
assert state.attributes['color_temp'] == 400
"""Basic checks for HomeKitLock."""
from tests.components.homekit_controller.common import (
FakeService, setup_test_component)
LOCK_CURRENT_STATE = ('lock-mechanism', 'lock-mechanism.current-state')
LOCK_TARGET_STATE = ('lock-mechanism', 'lock-mechanism.target-state')
def create_lock_service():
"""Define a lock characteristics as per page 219 of HAP spec."""
service = FakeService('public.hap.service.lock-mechanism')
cur_state = service.add_characteristic('lock-mechanism.current-state')
cur_state.value = 0
targ_state = service.add_characteristic('lock-mechanism.target-state')
targ_state.value = 0
# According to the spec, a battery-level characteristic is normally
# part of a seperate service. However as the code was written (which
# predates this test) the battery level would have to be part of the lock
# service as it is here.
targ_state = service.add_characteristic('battery-level')
targ_state.value = 50
return service
async def test_switch_change_lock_state(hass, utcnow):
"""Test that we can turn a HomeKit lock on and off again."""
lock = create_lock_service()
helper = await setup_test_component(hass, [lock])
await hass.services.async_call('lock', 'lock', {
'entity_id': 'lock.testdevice',
}, blocking=True)
assert helper.characteristics[LOCK_TARGET_STATE].value == 1
await hass.services.async_call('lock', 'unlock', {
'entity_id': 'lock.testdevice',
}, blocking=True)
assert helper.characteristics[LOCK_TARGET_STATE].value == 0
async def test_switch_read_lock_state(hass, utcnow):
"""Test that we can read the state of a HomeKit lock accessory."""
lock = create_lock_service()
helper = await setup_test_component(hass, [lock])
helper.characteristics[LOCK_CURRENT_STATE].value = 0
helper.characteristics[LOCK_TARGET_STATE].value = 0
state = await helper.poll_and_get_state()
assert state.state == 'unlocked'
assert state.attributes['battery_level'] == 50
helper.characteristics[LOCK_CURRENT_STATE].value = 1
helper.characteristics[LOCK_TARGET_STATE].value = 1
state = await helper.poll_and_get_state()
assert state.state == 'locked'
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment