search

bernikr / lovelace-notify-card

Send notifications directly from the dashboard
MIT License
45 stars 16 forks source link

Merge config.data with tts object #17

Closed dmaciaszek closed 2 months ago

dmaciaszek commented 1 year ago

With tts mode there was no possibility to pass some additional data like language or cache. In this PR this function work.

bernikr commented 1 year ago

I like the changes in general, however I think it could be confusing that the data configuration is merged with the normal service call data for tts but used inside a data key for non-tts services. I don't want to change the existing way the data attribute is handled for non tts services, as that would be a breaking change.

So I see a few possibilities:

  1. Maybe there is a more descriptive name then data that could be used
  2. Dont use a separate key at all but instead merge the unused keys of the main config with the service call options.

Either way it should be implemented for non-tts-services as that would reduce the complexity of configuration options.

dmaciaszek commented 1 year ago

Hmm...I don't want to change the existing way the data attribute is handled for non tts services, as that would be a breaking change - I do not understand. Changes are only for tts domain. Non-tts services works as before.

smenzer commented 7 months ago

i agree with @dmaciaszek - this change is only for tts and it matches the way you call the service anyways. I was trying to add language: fr-fr under the data object thinking it would work since the docs say "if your service requires extra data, put it in data"... so I don't think people will be confused with this.

bernikr commented 2 months ago

this pr is not needed anymore as the card got an overhaul and works with any service now