Enhancement: Update device classes and Entity names

WhimsySpoon commented 6 months ago

Thank you so much for creating this integration. I've been using it for a couple of days and it's solved a number of issues for me.

I've made a couple of manual changes by overriding defaults that you may wish to consider:

Device Classes

binary_sensor.xyz_charging should have a device_class of battery_charging.
binary_sensor.xyz_bluetooth_audio_connected should have a device_class of device_class: connectivity
As above, but with the _audio_device entity
As above, but with the _online sensor

Entity Names

In line with other integrations, should the "Last Updated At" sensor simply be called "Last Updated"?
If the change to the bluetooth_audio_connected device class is made, the _connected could probably be removed from the name. Same with the _audio_device

Units of Measurement

Should sensor.xyz_ambient_light_reading have a unit_of_measurement of lux?

Other changes I have both a v2 and a v3 device. The v2 device doesn't seem to have an internal temperature sensor as the entity always reports 0. Could this be automatically disabled on the v2 devices, and presumably the v1 devices too if they are also missing one?

I'm installing the new versions as soon as they are made available, but would be happy to test any changes or help in other ways.

cjb0001 commented 6 months ago

Other changes I have both a v2 and a v3 device. The v2 device doesn't seem to have an internal temperature sensor as the entity always reports 0. Could this be automatically disabled on the v2 devices, and presumably the v1 devices too if they are also missing one?

I was going to suggest this too, but from the perspective of mini vs player. From what I've seen, and from your input:

	player			mini
	v1	v2	v3	mini
temp	no?	no	yes	no
nightlight			yes	no
ambientLightSensorReading			yes	no

'deviceType': 'mini', 'deviceFamily': 'mini'

WhimsySpoon commented 6 months ago

That table is a good idea.

I don't have a Player v1 or a mini, so can only speak for the Players v2 and v3.

The Player v2 does have an ambientLightSensorReading, ~~but does not have a nightlight~~

Edit: I'm an idiot - the Player v2 *DOES** have a night light. It looks like it's just missing the temp sensor.

cdnninja commented 6 months ago

Sensors that don't have data won't display. However I get the impression sometimes that data comes back as 0. In which case we will need to add more logic to fix that.

As for the device classes. It looks like I had a bunch of device classes set for binary sensors but wasn't passing them back to HA. I have since fixed that. Update from "master" in hacs to try these changes out. Or wait for the next version number.

WhimsySpoon commented 6 months ago

Thanks for updating. I've tried v1.10.5 and it's unable to load:

Logger: homeassistant.loader
Source: loader.py:1238
First occurred: 11:18:58 (2 occurrences)
Last logged: 11:18:59

Unexpected exception importing platform custom_components.yoto.sensor
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/loader.py", line 1238, in _load_platform
    cache[full_name] = self._import_platform(platform_name)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/loader.py", line 1270, in _import_platform
    return importlib.import_module(f"{self.pkg_path}.{platform_name}")
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/util/loop.py", line 144, in protected_loop_func
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/importlib/__init__.py", line 90, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1387, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1360, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1331, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 935, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 995, in exec_module
  File "<frozen importlib._bootstrap>", line 488, in _call_with_frames_removed
  File "/config/custom_components/yoto/sensor.py", line 50, in <module>
    native_unit_of_measurement=UnitOfTemperature.LIGHT_LUX,
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: type object 'UnitOfTemperature' has no attribute 'LIGHT_LUX'

cjb0001 commented 6 months ago

Sensors that don't have data won't display. However I get the impression sometimes that data comes back as 0. In which case we will need to add more logic to fix that.

This is what it looks like for a mini. As you can see, the sensor values come back at 0. To me it's a low priority, but to build the perfect integration I suppose only the relevant things should show for each model.

WhimsySpoon commented 6 months ago

It looks like changing native_unit_of_measurement=UnitOfTemperature.LIGHT_LUX to native_unit_of_measurement=LIGHT_LUX in sensor.py (line 50) fixes the issue.

I've made the change to the file directly on my instance and it's working. I'll raise a PR when I'm home from work.

cdnninja commented 6 months ago

Let me know if looks better now. Temp should also be gone for you.

WhimsySpoon commented 6 months ago

Temp is still there on v2 player unfortunately. I upgrade via HACS, deleted the integration and reinstalled it. The Temp sensor came back with the re-install.

cjb0001 commented 5 months ago

Here's the updated table for the logic of displaying or not the three sensors. If anyone can comment on v1 that would be great.

	player			mini
	v1	v2	v3	mini
temp	no?	no	yes	no
nightlight		yes	yes	no
ambientLightSensorReading		yes	yes	no

'deviceType': 'mini', 'deviceFamily': 'mini'
'deviceType': 'v3', 'deviceFamily': 'v3'

cdnninja commented 5 months ago

Would you be interested in helping with docs for this? Would love help on that front too.

WhimsySpoon commented 5 months ago

I'll gladly help with the docs. I'll try and get a PR raised later today with the version/compatibility information.

cdnninja commented 5 months ago

Could someone who doesn't have a temp value check to see if it still shows for them? These lines should not add the value if it is 0. Not sure why that isn't working.

https://github.com/cdnninja/yoto_api/blob/a2a9f766014ef9639a0098fe385026a65dcee46b/yoto_api/YotoAPI.py#L117-L120

lntrpdx commented 5 months ago

Hi, I have a yoto v2 and can still see the temp sensor (with a value set to 0) even if it doesn't exist on the v2 version

cdnninja commented 5 months ago

Could you check what value is coming back from the player in the logs for temp? I assumed 0 but maybe something slightly different.

WhimsySpoon commented 5 months ago

Here you go. Looks like it's coming through as 0.0, which would explain why it's not being caught as 0.

'temp': '0:0'

Get Device Config Response: {
   "device":{
      "deviceId":"y2f4BaktzJvyNwSwRAcPRX6j",
      "name":"Harry's Yoto Player",
      "errorCode":"None",
      "fwVersion":"v2.17.7",
      "popCode":"xyz",
      "releaseChannelId":"beta",
      "releaseChannelVersion":"v2.17.7",
      "activationPopCode":"XYZ",
      "registrationCode":"XYZ",
      "deviceType":"v2",
      "deviceFamily":"v2",
      "deviceGroup":"",
      "mac":"XYZ",
      "online":true,
      "geoTimezone":"Europe/London",
      "getPosix":"GMT0BST,M3.5.0/1,M10.5.0",
      "status":{
         "activeCard":"eBpqG",
         "aliveTime":"None",
         "als":4095,
         "battery":"None",
         "batteryLevel":66,
         "batteryRemaining":"None",
         "bgDownload":0,
         "bluetoothHp":0,
         "buzzErrors":0,
         "bytesPS":0,
         "cardInserted":1,
         "chgStatLevel":"None",
         "charging":0,
         "day":1,
         "dayBright":"None",
         "dbatTimeout":"None",
         "dnowBrightness":"None",
         "deviceId":"XYZ",
         "errorsLogged":15,
         "failData":"None",
         "failReason":"None",
         "free":"None",
         "free32":"None",
         "freeDisk":12907456,
         "freeDMA":"None",
         "fwVersion":"v2.17.7",
         "headphones":0,
         "lastSeenAt":"None",
         "missedLogs":"None",
         "nfcErrs":"n/a",
         "nightBright":"None",
         "nightlightMode":"off",
         "playingStatus":0,
         "powerCaps":"None",
         "powerSrc":0,
         "qiOtp":"None",
         "sd_info":"None",
         "shutDown":"None",
         "shutdownTimeout":"None",
         "ssid":"xyz",
         "statusVersion":"None",
         "temp":"0:0",
         "timeFormat":"None",
         "totalDisk":15441920,
         "twdt":0,
         "updatedAt":"2024-05-26T16:08:25.975Z",
         "upTime":14,
         "userVolume":50,
         "utcOffset":3600,
         "utcTime":1716739704,
         "volume":27,
         "wifiRestarts":"None",
         "wifiStrength":-64
      },
      "config":{
         "locale":"en",
         "bluetoothEnabled":"0",
         "repeatAll":true,
         "showDiagnostics":true,
         "btHeadphonesEnabled":true,
         "pauseVolumeDown":false,
         "pausePowerButton":false,
         "displayDimTimeout":"0",
         "shutdownTimeout":"0",
         "headphonesVolumeLimited":false,
         "dayTime":"07:30",
         "maxVolumeLimit":"16",
         "ambientColour":"#ff8500",
         "dayDisplayBrightness":"auto",
         "dayYotoDaily":"3nC80/daily/<yyyymmdd>",
         "dayYotoRadio":"3nC80/radio-day/01",
         "daySoundsOff":"0",
         "nightTime":"19:00",
         "nightMaxVolumeLimit":"3",
         "nightAmbientColour":"#41c0f0",
         "nightDisplayBrightness":"4",
         "nightYotoDaily":"3nC80/daily/<yyyymmdd>",
         "nightYotoRadio":"3nC80/radio-night/01",
         "nightSoundsOff":"1",
         "hourFormat":"24",
         "displayDimBrightness":"0",
         "systemVolume":"70",
         "volumeLevel":"safe",
         "clockFace":"digital-sun",
         "logLevel":"none",
         "alarms":[

         ]
      },
      "shortcuts":{
         "versionId":"36645a9463e038d6cb9923257b38d9d9df7a6509",
         "modes":{
            "day":{
               "content":[
                  {
                     "cmd":"track-play",
                     "params":{
                        "card":"3nC80",
                        "chapter":"daily",
                        "track":"<yyyymmdd>"
                     }
                  },
                  {
                     "cmd":"track-play",
                     "params":{
                        "card":"3nC80",
                        "chapter":"radio-day",
                        "track":"01"
                     }
                  }
               ]
            },
            "night":{
               "content":[
                  {
                     "cmd":"track-play",
                     "params":{
                        "card":"3nC80",
                        "chapter":"daily",
                        "track":"<yyyymmdd>"
                     }
                  },
                  {
                     "cmd":"track-play",
                     "params":{
                        "card":"3nC80",
                        "chapter":"radio-night",
                        "track":"01"
                     }
                  }
               ]
            }
         }
      }
   }
}

Edit: formatting

cdnninja commented 5 months ago

The value I am using is in the "Get Device Status Response:" logs. It is the value of temperatureCelcius. Could you check that one? The 0:0 is interesting though. Mine reports 0:21. 21 being the temp in degree C.

Where as for me temperatureCelcius returns 21, easier to use.

WhimsySpoon commented 5 months ago

I've re-loaded the integration, and in the Get Device Status Response for the v2 device, that value is:

'temperatureCelcius': '0'

cdnninja commented 5 months ago

Turns out it was a string (text) vs a number in my code. Should be fixed in master branch. That leaves night light and ambient light sensing to remove for those players.

Anything else for this thread outstanding or do the device classes and names look okay now?

WhimsySpoon commented 5 months ago

Brilliant. I did wonder last night if it was a simple issue of a data type problem.

We've since discovered that the v2 Player does support nightlight and ambient light sensor (https://github.com/cdnninja/yoto_ha/issues/53#issuecomment-2129205677), so they can remain. Unsure about the v1 player as I don't think we've seen anyone with that model to confirm.

I've installed master and the v2 temp reading is now unavailable. Deleting and re-adding the integration resulted in the temp sensor not being added for the v2 device at all. Excellent! It remains correct on my v3 device as expected.

The issue with the _media_player entity name still exists (https://github.com/cdnninja/yoto_ha/issues/49), otherwise it looks great!

Unless there are any objections, I think we can close this issue now.

cdnninja / yoto_ha

Enhancement: Update device classes and Entity names #53