Fix inconsistent HDD mapping

Dimand commented 9 months ago

Proposed change

Addresses bug https://github.com/tomaae/homeassistant-truenas/issues/77 As discussed in the bug, how to name drives is a bit arbitrary but the API returns identifier that is a combination of the drives serial number and the LUN ID. As far as I know this should never be blank even for drives that wont return a serial string.

Drive entity ID will no longer be in the form sensor.devname, example: "sensor.sda". New format is sensor.serial_lunid_SERIAL_LUNID, example: sensor.serial_lunid_WD123XYZ_5000c500c38428e

Drives can easily be renamed to more sensible things as desired by the user.

Type of change

[x] Bugfix
[ ] New feature
[ ] Code quality improvements to existing code or addition of tests
[ ] Documentation

Additional information

Only tested with TrueNAS Scale. Would be great if someone could check it on Core.

Checklist

[x] The code change is tested and works locally.
[x] The code has been formatted using Black.
[ ] Tests have been added to verify that the new code works.
[ ] Documentation added/updated if required.

tomaae commented 9 months ago

hmm, current format is "sensor.disks" if we use lun, it should be "sensor.disks" how did you get it to show as "sensor.devname"?

I will test it on my live core later, right now I'm replacing several disks and extending my nas, so it could mess up with testing as I shuffle disks around.

Dimand commented 8 months ago

Perhaps this is a difference between core and scale? I am running and testing on scale. The current version uses devname as the entity_id, but this is identical to name for me at least. https://github.com/tomaae/homeassistant-truenas/blob/1918a8da0378f2c64b2d81498cc96d1fdc35f4ac/custom_components/truenas/sensor_types.py#L370 I changed this to get identifier as the entity_id in HA. https://github.com/tomaae/homeassistant-truenas/blob/f7aa61151df0e3489dd1a2ffd9ed3981db014d12/custom_components/truenas/sensor_types.py#L374

This is also what is returned by the temps API as an identifier. See response to /disk/temperatures api request below. So this is why the temp update code now uses the vals["name"] attribute to assign the correct temp.

I would have assumed in core this would be the freeBSD ad0, ad1 etc.

For reference here is a /disk api return in scale where name is derived from devname.

tomaae commented 8 months ago

ok, I will have to look more in detail on how new scale display information. looking at api putput, it may make more sense to use lunid instead of identifier attribute. I assume it could be disk guid too.

Dimand commented 8 months ago

Happy for you to change anything/everything to be more sensible.

I did initially start using zfs_guid but that is not set unless the drive is in a pool and will return null. I don't really know enough about how lunid is assigned and maintained to be sure that it will stay consistent but it appears to work. I think this is related - https://en.wikipedia.org/wiki/World_Wide_Name.

output example for /disk without zfs_guid:

Thanks for looking into it and thanks again for the integration! My aircon now turns on when my drives get too hot 👍.

tomaae commented 8 months ago

ah yea, when drive is not formatted for zfs, it would not work zfs guid. I will test it in detail in few days, once I'm done messing with my NAS and all resilvering is done.

tomaae / homeassistant-truenas