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

Fix inconsistent HDD mapping #131

Merged
merged 2 commits into from
Mar 1, 2024
Merged

Fix inconsistent HDD mapping #131

merged 2 commits into from
Mar 1, 2024

Conversation

Dimand
Copy link
Contributor

@Dimand Dimand commented Dec 16, 2023

Proposed change

Addresses bug #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.
image

image

Type of change

  • 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

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

Keeps consentient disk identifiers through reboots. Uses a combination of serial and LUN ID. Should always be populated and unique.
@tomaae
Copy link
Owner

tomaae commented Dec 16, 2023

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
Copy link
Contributor Author

Dimand commented Dec 16, 2023

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.


I changed this to get identifier as the entity_id in HA.

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.

image

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.

image

@tomaae
Copy link
Owner

tomaae commented Dec 17, 2023

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
Copy link
Contributor Author

Dimand commented Dec 17, 2023

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:
image

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

@tomaae
Copy link
Owner

tomaae commented Dec 17, 2023

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.

@github-actions github-actions bot added the stale label Jan 1, 2024
@github-actions github-actions bot closed this Jan 9, 2024
@tomaae tomaae reopened this Jan 9, 2024
Repository owner deleted a comment from github-actions bot Jan 9, 2024
@github-actions github-actions bot removed the stale label Jan 10, 2024
@github-actions github-actions bot added the stale label Jan 24, 2024
Repository owner deleted a comment from github-actions bot Jan 25, 2024
@tomaae tomaae removed the stale label Jan 25, 2024
@github-actions github-actions bot added stale and removed stale labels Feb 9, 2024
Repository owner deleted a comment from github-actions bot Feb 15, 2024
@github-actions github-actions bot added the stale label Mar 1, 2024
Repository owner deleted a comment from github-actions bot Mar 1, 2024
@tomaae tomaae merged commit 50a5bb7 into tomaae:master Mar 1, 2024
13 of 15 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants