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

feat: add new attribute: previous_volume #2766

Merged
merged 6 commits into from
Dec 22, 2024

Conversation

danielbrunt57
Copy link
Collaborator

@danielbrunt57 danielbrunt57 commented Dec 19, 2024

This saves the current volume level in new attribute previous_volume before changing the volume level.

See discussion #2765

danielbrunt57 and others added 2 commits December 18, 2024 19:48
This saves the current volume level as previous_volume before changing it.
@coveralls
Copy link

coveralls commented Dec 19, 2024

Pull Request Test Coverage Report for Build 12411900097

Details

  • 0 of 6 (0.0%) changed or added relevant lines in 1 file are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.008%) to 9.349%

Changes Missing Coverage Covered Lines Changed/Added Lines %
custom_components/alexa_media/media_player.py 0 6 0.0%
Files with Coverage Reduction New Missed Lines %
custom_components/alexa_media/media_player.py 2 0.0%
Totals Coverage Status
Change from base Build 12365852297: -0.008%
Covered Lines: 320
Relevant Lines: 3423

💛 - Coveralls

@danielbrunt57
Copy link
Collaborator Author

danielbrunt57 commented Dec 19, 2024

@alandtse Any idea what's triggering codespell exit code 65?

@alandtse
Copy link
Owner

alandtse commented Dec 19, 2024

You can't read the log? It literally points out the misspelling.

custom_components/alexa_media/media_player.py:1134: curent ==> current

@danielbrunt57
Copy link
Collaborator Author

Duh! I see it now plain as day now that you pointed it out!

@danielbrunt57
Copy link
Collaborator Author

FYI, I just completed creating a new service to restore the previous volume level and will be my next PR to supplement this one.
I think it will be a welcome addition rather than having to use a templated service call to media_player.set_volume.

image

    @_catch_login_errors
    async def restore_volume(self, call) -> bool:
        """Handle restore volume service request.

        Arguments
            call.ATTR_ENTITY_ID {str: None} -- Alexa Media Player entity.

        """
        entity_id = call.data.get(ATTR_ENTITY_ID)
        _LOGGER.debug("Service restore_volume called for: %s", entity_id)

        # Retrieve the entity registry and entity entry
        entity_registry = er.async_get(self.hass)
        entity_entry = entity_registry.async_get(entity_id)

        if not entity_entry:
            _LOGGER.error("Entity %s not found in registry", entity_id)
            return False

        # Retrieve the previous volume from the entity's state attributes
        state = self.hass.states.get(entity_id)
        if not state or 'previous_volume' not in state.attributes:
            _LOGGER.error("Previous volume attribute not found for entity %s", entity_id)
            return False

        previous_volume = state.attributes['previous_volume']

        # Call the volume_set service with the retrieved volume
        await self.hass.services.async_call(
            domain="media_player",
            service="volume_set",
            service_data={
                "volume_level": previous_volume,
            },
            target={"entity_id": entity_id},
        )

        _LOGGER.debug("Volume restored to %s for entity %s", previous_volume, entity_id)
        return True

@danielbrunt57
Copy link
Collaborator Author

danielbrunt57 commented Dec 19, 2024

I don't know where else I can post for your feedback so here goes...

I also have a somewhat major revision re: include_devices and exclude_devices and I'm wondering if that should be a beta release to thresh out any issues?

I originally was going to separate binary_sensors/lights/sensors/switches but I may just leave them in selected_devices so that will need cleanup.

image

The new code uses a flag for include, exclude or none and stores selected_devices as a list.
It also incorporates "migration" from the legacy keys (which I tested):

    # Migration logic
    if CONF_INCLUDE_DEVICES in account or CONF_EXCLUDE_DEVICES in account:
        _LOGGER.info("Migrating legacy include/exclude devices configuration.")
        
        # Default values
        filter_mode = "none"  # Default to 'none'
        selected_devices = []

        # Check exclude devices first (it takes priority)
        exclude_devices = account.get(CONF_EXCLUDE_DEVICES, "").strip()
        include_devices = account.get(CONF_INCLUDE_DEVICES, "").strip()

        if exclude_devices:  # Prioritize exclude devices
            filter_mode = "exclude"
            selected_devices = cv.ensure_list_csv(exclude_devices)
            _LOGGER.debug("Migrating exclude devices: %s", selected_devices)
        elif include_devices:  # Use include devices only if exclude is empty
            filter_mode = "include"
            selected_devices = cv.ensure_list_csv(include_devices)
            _LOGGER.debug("Migrating include devices: %s", selected_devices)

        # Build the updated config data
        new_data = {
            **account,
            CONF_SELECTED_DEVICES: selected_devices,
            CONF_FILTER_MODE: filter_mode,
        }

        # Remove legacy keys
        new_data.pop(CONF_INCLUDE_DEVICES, None)
        new_data.pop(CONF_EXCLUDE_DEVICES, None)

        # Update the config entry
        hass.config_entries.async_update_entry(config_entry, data=new_data)
        _LOGGER.info("Migration completed successfully: %s", new_data)

@danielbrunt57
Copy link
Collaborator Author

danielbrunt57 commented Dec 19, 2024

And one more...

I have revised my alexapy to incorporate throttling based on region and uses semaphores to limit concurrent requests and also to instill a delay between each request.

alexaapi.py:

    _semaphore_map: dict[str, asyncio.Semaphore] = {}
    _last_request_times: dict[str, float] = {}
    _region_limits = {"eu": 2, "fe": 3, "na": 4}
    _region_delays = {"eu": 0.9, "fe": 0.7, "na": 0.5}

Before I submit that, I'd like to try and figure out a way to make them user configurable.

Most of the requests (for me) are fine but the get_history_records is being a pain...

@alandtse alandtse merged commit 330e046 into alandtse:dev Dec 22, 2024
4 checks passed
@popy2k14
Copy link

@danielbrunt57 thx a lot for keeping the project alive.

Does this feature also work for storing the music/stream volume before an announcement?

I am heavily using announcements and have made a script which stores the volume of current playing media and restores it after it.

Would be nice if this would get easier with an addition like this.

@danielbrunt57
Copy link
Collaborator Author

danielbrunt57 commented Dec 22, 2024

Does this feature also work for storing the music/stream volume before an announcement?

Are you using a media_player.volume_set action to set the volume before the announcement is made?

If the volume is changed by a "Media player: volume" action/service call, previous_volume should be updated to the value of the volume_level attribute before the new value is set into volume_level after which the state of the media player entity with the new values is updated.
Currently, it does retain the volume_level as previous_volume if the volume was changed by the volume buttons on the device which is updated in HA via http2push.

I'd watch the attributes of the media player entity in Dev Tools > States to see what is changing as you manually call the actions that your automations/scripts are performing.
image

@popy2k14
Copy link

Yeah, i am using volume_set, here is my script, now modified to use your new "previous_volume" attribute:

alias: Alexa say something
description: Say something with an Alexa device (tts or announce)
variables:
  pause: "{{ 1.7 if type == 'tts' else 0.0 | float }}"
  gong: "{{ 1.7 if type == 'announce' else 0.0 | float }}"
  dur: "{{ duration | default(7, true) | float }}"
fields:
  message:
    description: the message (SSML)
    example: <amazon:effect name="whispered">Please close the door.</amazon:effect>
    required: true
  type:
    description: tts or announce
    example: tts
    required: true
  target:
    description: the target device as a string
    example: media_player.kuche
    required: true
  volume:
    description: the volume of the speach
    example: 0.7
    required: true
  duration:
    description: the length of the speach in seconds
    example: 3.3
    required: true
sequence:
  - data:
      target: "{{ target }}"
      message: >-
        <break time='{{ pause + 0.2 | float }}s'/> {{ message }} <break
        time='1s'/>
      data:
        type: "{{ type }}"
    enabled: true
    action: notify.alexa_media
  - delay:
      hours: 0
      minutes: 0
      seconds: "{{ pause | float }}"
      milliseconds: 0
    enabled: true
  - data:
      volume_level: "{{ volume }}"
    target:
      entity_id: "{{ target }}"
    enabled: true
    action: media_player.volume_set
  - delay:
      hours: 0
      minutes: 0
      seconds: "{{ (gong + dur) | float }}"
      milliseconds: 0
    enabled: true
  - variables:
      reset_volume: >-
        {{ state_attr(target, 'previous_volume') | default(0.35, true) | float
        }}
  - data:
      volume_level: "{{ reset_volume | float }}"
    target:
      entity_id: "{{ target }}"
    enabled: true
    action: media_player.volume_set
  - repeat:
      sequence:
        - delay:
            hours: 0
            minutes: 0
            seconds: 1
            milliseconds: 0
      until:
        - condition: template
          value_template: >-
            {{ (reset_volume | default(1, true) | float) == (state_attr(target,
            'volume_level') | default(2, true) | float)  or (repeat.index ==
            10)}}
    enabled: true
mode: queued
max: 5
icon: mdi:bullhorn

All is working great, thanks a lot for this addition.
Cant wait for your new teased service here: #2766 (comment)
This would make it much more usable for the great HA community, rather than everyone is using an script to automate tts/annoucements!

Maybe you could take insparation by the script above :-)
The script is taking the "gong" before an anouncement and a customizeable time into account before reverting the volume.
Working really good here.

@danielbrunt57
Copy link
Collaborator Author

Cant wait for your new teased service here: #2766 (comment)

See PR #2773

@popy2k14
Copy link

thx a lot for the two PR's "storing last volume" and "service to restore".
The icing on the cake would be an tts/announce with an bool flag which takes care of the whole volume store, set, reset (like the automation above).

@danielbrunt57
Copy link
Collaborator Author

The icing on the cake would be an tts/announce with an bool flag which takes care of the whole volume store, set, reset (like the automation above).

The problem with your request for "icing" on the cake is that one of the ingredients for that "icing" comes from a different store!
You would need to be able to specify the desired volume level for the announcement via the service call but that service call is defined in Home Assistant Core Components: Notify.

https://github.com/home-assistant/core/blob/dev/homeassistant/components/notify/services.yaml:

# Describes the format for available notification services

notify:
  fields:
    message:
      required: true
      example: The garage door has been open for 10 minutes.
      selector:
        text:
    title:
      example: "Your Garage Door Friend"
      selector:
        text:
    target:
      example: platform specific
      selector:
        object:
    data:
      example: platform specific
      selector:
        object:

https://github.com/home-assistant/core/blob/dev/homeassistant/components/notify/strings.json:

  "services": {
    "notify": {
      "name": "Send a notification",
      "description": "Sends a notification message to selected targets.",
      "fields": {
        "message": {
          "name": "Message",
          "description": "Message body of the notification."
        },
        "title": {
          "name": "Title",
          "description": "Title for your notification."
        },
        "target": {
          "name": "Target",
          "description": "Some integrations allow you to specify the targets that receive the notification. For more information, refer to the integration documentation."
        },
        "data": {
          "name": "Data",
          "description": "Some integrations provide extended functionality. For information on how to use _data_, refer to the integration documentation."
        }
      }
    },

@popy2k14
Copy link

@danielbrunt57 maybe we could use "data" for this? Until the core is changed with an volume float?

@danielbrunt57
Copy link
Collaborator Author

danielbrunt57 commented Dec 29, 2024

@popy2k14 Possibly! I'm kicking that one around and going to try some code in notify.py.
I still have code I need to figure out.

@popy2k14
Copy link

That would be really nice!
Just let me know if you need someone to test.

@danielbrunt57
Copy link
Collaborator Author

That would be really nice! Just let me know if you need someone to test.

I started a new discussion...

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 this pull request may close these issues.

4 participants