Lametric component sound problem and cycles feature request

[frittes]

Home Assistant release (hass --version):

0.57.3

Python release (python3 --version):

Python 3.5.2

Component/platform:

https://home-assistant.io/components/notify.lametric/

Description of problem:

Notification sound does not work.

Additional info:

Added

sound: notification

to /home/homeassistant/.homeassistant/notify.yaml

- name: lametric1
  platform: lametric
  display_time: 20
  icon: a7956
  sound: notification

and component stops working.

In the code (lametric.py), the sound is added to the list of frames. And the list of frames is added to the model.

I think the sound has to be added to the model.

See:

https://github.com/keans/lmnotify/blob/master/lmnotify/models.py

and

http://lametric-documentation.readthedocs.io/en/latest/reference-docs/device-notifications.html

notify/lametric.py

        frames = [text_frame]

        if sound is not None:
            frames.append(sound)

        _LOGGER.debug(frames)

        model = Model(frames=frames)

If sound is added to the list of frames, component stops working.

It would be nice, if cycles were also implemented. At the moment, the notification is displayed once, also if display_time is set to 20 seconds. If cycle are configurable, the notification could be shown maybe 3 times or 0 for static.

[MartinHjelmare]

Does the display_time option work? If display_time is set to 20 seconds, the notification is shown for 20 seconds?

[frittes]

No. The notification is only displayed once.

On the lametric site, the lifetime should be in milliseconds. In HA is in seconds.

[MartinHjelmare]

So nothing changes if you change the display_time option? Should we remove that option then?

[frittes]

Let me try in milliseconds.

[frittes]

It makes 20000000 in the log, the recalculate works. :D No, if it is set to 20 seconds in the config, the notification is showed once. It is about 6 seconds.

From lametric site for lifetime:

The time notification lives in queue to be displayed in milliseconds. Default lifetime is 2 minutes. If notification stayed in queue for longer than lifetime milliseconds – it will not be displayed.

It is not the time to display. It defaults to 2 minutes. I think it is useless.

[MartinHjelmare]

Thanks for this info. I think we should keep it to not make a breaking change, but we should document it properly with the info above.

I'll try to add cycles. Should it be a configuration option or service data option?

[frittes]

For my opinion it should be a configuration option. It lets the user choose how often a message repeats.

[frittes]

@MartinHjelmare The service call from the dev console works, but if I add the sound to the automation, it does not work.

`2017-11-14 20:25:36 INFO (MainThread) [homeassistant.core] Bus:Handling <Event call_service[L]: service_call_id=140511511142072-84, domain=notify, service=lametric1, service_data=sound=bicycle, message=Lennart ist unterwegs.>
2017-11-14 20:25:36 DEBUG (MainThread) [homeassistant.components.websocket_api] WS 140510057829264: Sending {'id': 2, 'type': 'event', 'event': {'time_fired': datetime.datetime(2017, 11, 14, 19, 25, 36, 424787, tzinfo=), 'event_type': 'state_changed', 'origin': 'LOCAL', 'data': {'new_state': <state device_tracker.lennart_iphone=not_home; longitude=XXXXX, friendly_name=Lennart, battery=100, tid=LE, entity_picture=/local/images/lennart_neu.png, latitude=XXXXX, source_type=gps, gps_accuracy=65 @ 2017-11-14T20:25:36.424747+01:00>, 'entity_id': 'device_tracker.lennart_iphone', 'old_state': <state device_tracker.lennart_iphone=home; longitude=XXXXX, friendly_name=Lennart, battery=100, tid=LE, entity_picture=/local/images/lennart_neu.png, latitude=XXXXX, source_type=gps, gps_accuracy=65 @ 2017-11-14T20:23:05.278210+01:00>}}}
2017-11-14 20:25:36 ERROR (MainThread) [homeassistant.core] Invalid service data for notify.lametric1: extra keys not allowed @ data['sound']. Got 'bicycle'

[frittes]

@MartinHjelmare Automation rule is the right place, isn't it?

I think, the best way would be, to make the definition of the sound in the config (notify.yaml or configuration.yaml)). One sound for all messages to lametric. Otherwise you have to add the sound to every notification and in my opinion there is no need to make different sounds for notifications on lametric from HA.

[MartinHjelmare]

I think we should keep the sound in the service call. That is the most flexible.

Can you post your automation? I think you're missing the data key in the service data. It's easier to test service calls in the developer tools service tab in the frontend. There you write json instead of yaml though.

[frittes]

This is my automation rule:

- alias: 'Sende Nachricht beim Verlassen des Zuhauses von Lennart'
  trigger:
    platform: state
    entity_id: device_tracker.lennart_iphone
    from: 'home'
    to: 'not_home'
  action:
    service: notify.lametric1
    data:
      message: 'Lennart ist unterwegs.'
      sound: 'bicycle'

[MartinHjelmare]

Try this:

- alias: 'Sende Nachricht beim Verlassen des Zuhauses von Lennart'
  trigger:
    platform: state
    entity_id: device_tracker.lennart_iphone
    from: 'home'
    to: 'not_home'
  action:
    service: notify.lametric1
    data:
      message: 'Lennart ist unterwegs.'
      data:
        sound: 'bicycle'

[frittes]

Failed config on config test.

[frittes]

My fault. With data...data...it works.

[frittes]

But it is another way like mentioned in the automation documentation of action. "You have to do it like this, but with lametric you have to go another way..."

Is it possible to make it like:

- alias: 'Sende Nachricht beim Verlassen des Zuhauses von Lennart'
  trigger:
    platform: state
    entity_id: device_tracker.lennart_iphone
    from: 'home'
    to: 'not_home'
  action:
    service: notify.lametric1
    data:
      message: 'Lennart ist unterwegs.'
      sound: 'bicycle'

For me it works, but usability?

[MartinHjelmare]

No the data key under service data for notify component is made for non standard extra keys, eg sound in this case.

This is documented here:
https://home-assistant.io/components/notify/#service

[frittes]

Ok, then it has to be documented in the lametric component (Extended functionality).

Do we rock on with the cycles change?

[MartinHjelmare]

Yeah, I'll try do something on the cycles. I'll ping you when I have something.

[frittes]

@MartinHjelmare Made it and created a pull request.

#10656