Skip to content

Default name for Tapo P304M that can't be named #141069

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

Closed
wants to merge 1 commit into from

Conversation

nicosemp
Copy link

Proposed change

Tapo P304M is a power strip with 4 sockets that can be controlled individually.

Since the Tapo P304M can't be named in the Tapo app, entities are created with unnamed_p304m_... as id prefix. Only the four sockets can be renamed (default is "Tapo Plug 1" with numbers 1 to 4). Renaming 64 entity ids by hand is painful, so a default name like "Tapo P304M" would be nice. It would also result in id prefixes being tapo_p304m_....

This PR is a draft to explore the possibilities to achieve the desired result.
Currently the only change is inside the get_device_name method. If the device has no alias and parent is falsy, then I added an additional check for device.model == "P304M" to return a default "Tapo P304M".

Critical points I'm looking for feedback on:

  • This solves the entity ids problem, as they all start with tapo_p304m_... now. However I'm not sure if this is the right chunk of code to accomplish this. Maybe it should be done wherever this device_name is turned to snake_case and used to assemble the entity ids.
  • The Entity name is still "None P304M". This can be easily renamed from the frontend, but it would be nice to find where it's set and use a similar if check to set the same string as a default.

This aims to be a non-breaking fix.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

@home-assistant
Copy link

Hey there @rytilahti, @bdraco, @sdb9696, mind taking a look at this pull request as it has been labeled with an integration (tplink) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of tplink can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign tplink Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

@nicosemp nicosemp marked this pull request as ready for review March 21, 2025 18:34
Comment on lines +286 to +290
# The P304M is a power strip with no alias. During setup in the Tapo app the parent
# device cannot be named, only the individual outlets can have aliases so we need to
# return a valid one for the parent to avoid "unnamed_p304m" in the entity id.
if device.model == "P304M":
return "Tapo P304M"
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, this makes me wonder, should the device model be the final fallback in general?

Copy link
Member

Choose a reason for hiding this comment

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

Probably yes, combined with a shortened device id, or a mac. I'd prefer to have that implemented upstream, so feel free to create a PR to python-kasa project.

Copy link
Author

Choose a reason for hiding this comment

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

@frenck I agree it's not ideal, I just wanted to start a conversation to understand where the solution should live. @rytilahti I'm not familiar with that API. Thank you both for the feedback!

As mentioned in this PR's description, it would be best to fix both both the entity ids and the device name. The latter is set to "None P304M" automatically. It can easily be changed in the UI, but "Tapo P304M" sounds like a better default name.

Could you point to where it's best to make the change to fix both problems on the python-kasa project?

Copy link
Member

@rytilahti rytilahti Apr 1, 2025

Choose a reason for hiding this comment

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

https://github.com/python-kasa/python-kasa/blob/579fd5aa2add10a2f3cc3ce21cb6a006e11de318/kasa/smart/smartdevice.py#L599 this is where the none is coming, but I cannot recall why it was marked as optional. Maybe to allow downstream projects to decide how to handle naming?

I'm not sure if we want to add the brand name to the alias, but that would be the place to modify the name (and indirectly, entity ids which are based on that) that is used by homeassistant and the kasa cli tool, too.

Copy link
Author

@nicosemp nicosemp Apr 4, 2025

Choose a reason for hiding this comment

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

Thanks for the direction! I created a PR with a proposed solution, you can read the short description and we can discuss further if necessary.

EDIT: I'll close this PR as soon as we're done on python-kasa.

@home-assistant
Copy link

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

Copy link

github-actions bot commented Jun 3, 2025

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes.
Thank you for your contribution!

@github-actions github-actions bot added the stale label Jun 3, 2025
@github-actions github-actions bot closed this Jun 10, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Jun 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tapo P304M entities are always registered as unnamed_p304m_name_of_entity
4 participants