Explicitly set entity name for VenstarSensor #102158
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
home-assistant
bot
added
bugfix
cla-signed
Hacktoberfest
integration: venstar
small-pr
|
Hey there @garbled1, @jhollowe, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
rainepretorius
approved these changes
Oct 17, 2023
joostlek
requested changes
Oct 17, 2023
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
I just checked the code, and every sensor does have a name, so I am curious where this message comes from |
|
So to explain it more, this message only pops up when the entity has @property
def name(self):
"""Return the name of the device."""
return self.entity_description.name_fn(self.sensor_name)This part of the code makes sure the code always returns a name. This function never seems to return I am guessing you can set this integration up in a development environment. Can you maybe give it a shot to change that piece of code to this? @property
def name(self):
"""Return the name of the device."""
name = self.entity_description.name_fn(self.sensor_name)
print(name)
return nameThis would print out the name of the sensor before being returned. If I have to believe the code, this should never print |
|
FWIW, the proposed change did make the warning go away for me, did not generate any new errors, and didn't appear to affect functionality at all. I changed the diagnostic code to use logging instead of print() (and reverted my change). Here's what I get on startup... |
|
Heh, this doesn't make sense |
|
Right, I looked it up and I might found something. Can you try |
That seems to make everything happy. If I don't hear any counter arguments, I'l update the commit in a bit. |
|
I copied this from Unifi, so I would say go for it |
dseven
force-pushed
the
venstar-sensor-entity-name
branch
from
35b0cb8 to
8ed5b8d
Compare
Set entity name using _attr_name instead of a name property, to avoid warnings about name being implicitly set to None.
dseven
force-pushed
the
venstar-sensor-entity-name
branch
from
8ed5b8d to
3be0e64
Compare
joostlek
approved these changes
Oct 17, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed change
Address:
[homeassistant.helpers.entity] Entity None (<class 'homeassistant.components.venstar.sensor.VenstarSensor'>) is implicitly using device name by not setting its name. Instead, the name should be set to None, please create a bug report at https://github.com/home-assistant/core/issues?q=is%3Aopen+is%3Aissue+label%3A%22integration%3A+venstar%22
Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.To help with the load of incoming pull requests: