Add GPU sensor to Glances

[fhoekstra]

Proposed change

In python-glances-api release 0.5.0, Glances' GPU sensors are exposed so that Home Assistant can display and track them. This change is about adding these GPU sensors to Home Assistant. This fixes issue #74444

Type of change

• [ ] Dependency upgrade
• [ ] Bugfix (non-breaking change which fixes an issue)
• [ ] New integration (thank you!)
• [x] 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

• This PR fixes or closes issue: fixes #74444
• This PR is related to issue:
• Link to documentation pull request: https://github.com/home-assistant/home-assistant.io/pull/30481

Checklist

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

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

• [x] Documentation added/updated for www.home-assistant.io

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.
• [ ] Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

• [x] I have reviewed two other open pull requests in this repository.

[home-assistant[bot]]

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

Learn more about our pull request process.

[home-assistant[bot]]

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

Code owner commands

Code owners of glances 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 glances 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.

[fhoekstra]

I haven't been able to test locally so far as I am developing in VSCode DevContainer and haven't been able to expose my GPU to the DevContainer yet.

[fhoekstra]

EDIT: never mind, found that HA uses syrupy and found the syrupy docs

@engrbm87 This is almost done. ~~I have completed local testing but am struggling to add the GPU to the new style of snapshot testing. Could you point me in the right direction for that?~~ Manually editing the test_sensor.ambr file does not seem to work for me so far with this:

# name: test_sensor_states[sensor.0_0_0_0_gpu_0_nvidia_geforce_rtx_3080_temperature-state]
  StateSnapshot({
    'attributes': ReadOnlyDict({
      'device_class': 'temperature',
      'friendly_name': '0.0.0.0 GPU_0__NVIDIA GeForce RTX 3080 temperature',
      'state_class': <SensorStateClass.MEASUREMENT: 'measurement'>,
      'unit_of_measurement': <UnitOfTemperature.CELSIUS: '°C'>,
    }),
    'context': <ANY>,
    'entity_id': 'sensor.0_0_0_0_gpu_0_nvidia_geforce_rtx_3080_temperature',
    'last_changed': <ANY>,
    'last_updated': <ANY>,
    'state': 'unavailable',
  })
# ---

[joostlek]

@fhoekstra, run the tests with pytest ./tests/components/glances --snapshot-update it will generate the snapshots

[fhoekstra]

Thanks @joostlek , just did that. Wasn't that hard to find that syrupy is the library we use, would a PR for the docs with mentioning syrupy and snapshots be appreciated?

~~I added the e-mail above to my Github account according to the instructions of the cla-bot and rebased the branch, but the bot still does not seem happy.~~

Never mind, just needed to wait a minute, bot is happy now

[joostlek]

https://developers.home-assistant.io/docs/development_testing#snapshot-testing :)

[cohenchris]

What's the status on this? Seems it all checks out, just need somebody with write access to approve.

[joostlek]

I think you still need to generate the new snapshots pytest ./tests/components/glances --snapshot-update

[fhoekstra]

@joostlek You're right, I dropped a commit somewhere. I did generate them now, but they look wrong: a few sensors don't have the GPU part of the entity ID it seems. Converting this back to draft until I can fix this

[joostlek]

Did you generate translations beforehand?

[joostlek]

python -m script.translations develop --all

[fhoekstra]

I did, but nothing happened. Maybe I messed up the translation_keys (I copied them from existing sensors as much as possible).

EDIT: Temperature and fan speed look good, but the processor and memory usage don't. Ohh I see it, in the strings.json, those 2 miss the {sensor_label} template and that is why

[joostlek]

Something still feels off GPU_0__NVIDIA GeForce RTX 3080 processor usage doesnt look like a good name

[fhoekstra]

I think it's all good now, added some new custom translation keys for processor and memory usage with the {sensor_label} template so the GPU name is inserted there and the entities can be distinguished based on that.

[fhoekstra]

Shall I replace the (double) underscores with spaces? Or do you think "processor" does not fit here?

[joostlek]

Oh its getting the ID from the json. I don't really like the name like this (as we have the user friendly name, so I think its strange to use some GPU__0_). The only case where this would make sense if someone has 2 the same GPUs. I'm probably leaving this discussion for @engrbm87

[fhoekstra]

In f041750 , I put in a proposal for removing the GPU_0__ prefix if only a single GPU is present on a host. @engrbm87 , let me know what you think of it.

[fhoekstra]

How about this?

• If there is only 1 GPU present on a host, remove the GPU_0__ prefix GPU_0__Nvidia RTX 3080 -> Nvidia RTX 3080
• If there are multiple GPUs present on a host, turn the prefix into a suffix: GPU_0__Nvidia RTX 3080 -> Nvidia RTX 3080 (GPU 0) GPU_1__Nvidia RTX 3070Ti -> Nvidia RTX 3070Ti (GPU 1)

Advantages:

• no weird numbers when a host has only a single GPU, friendly_name is friendly
• entity IDs always start with the make and model of the GPU
• GPU index is present in entity ID if there are multiple GPUs
• Multiple identical GPUs can be told apart

Disadvantages:

• A GPU's entity ID changes if another GPU is added to a host
• ?

[cohenchris]

How about this?

• If there is only 1 GPU present on a host, remove the GPU_0__ prefix GPU_0__Nvidia RTX 3080 -> Nvidia RTX 3080
• If there are multiple GPUs present on a host, turn the prefix into a suffix: GPU_0__Nvidia RTX 3080 -> Nvidia RTX 3080 (GPU 0) GPU_1__Nvidia RTX 3070Ti -> Nvidia RTX 3070Ti (GPU 1)

Advantages:

• no weird numbers when a host has only a single GPU, friendly_name is friendly
• entity IDs always start with the make and model of the GPU
• GPU index is present in entity ID if there are multiple GPUs
• Multiple identical GPUs can be told apart

Disadvantages:

• A GPU's entity ID changes if another GPU is added to a host
• ?

An easy solution to your disadvantage - always place the "GPU X" suffix at the end, regardless of how many GPUs there are.

Seems there is nothing else outstanding with this PR, no?

[fhoekstra]

That's a great solution! I'll implement it as soon as I can make the time (been busy with a move).

[fhoekstra]

Thanks for your help @wittypluck ! @joostlek I am sorry for tagging you and for not updating the status of this PR appropriately in the last months but now it seems ready to merge. Rebased, all feedback addressed, rechecked the list: working locally, tests pass, formatter happy.

(venv) ➜ ~/Source/home-assistant-core (add_gpu_sensors_to_glances) ✔ pytest tests/components/glances 
Test session starts (platform: linux, Python 3.12.2, pytest 8.1.1, pytest-sugar 1.0.0)
rootdir: /home/freek/Source/home-assistant-core
configfile: pyproject.toml
plugins: socket-0.7.0, syrupy-4.6.1, pytest_freezer-0.4.8, anyio-4.3.0, github-actions-annotate-failures-0.2.0, aiohttp-1.0.5, xdist-3.4.0, timeout-2.3.1, sugar-1.0.0, requests-mock-1.11.0, respx-0.21.0, cov-5.0.0, asyncio-0.23.6, typeguard-4.2.1, unordered-0.6.0, mock-3.14.0, picked-0.5.0
asyncio: mode=Mode.AUTO
collected 16 items                                                                                                                                                                                                                                                   

 tests/components/glances/test_config_flow.py ✓✓✓✓✓✓✓✓                                                                                                                                                                                                  50% █████     
 tests/components/glances/test_init.py ✓✓✓✓✓✓                                                                                                                                                                                                           88% ████████▊ 
 tests/components/glances/test_sensor.py ✓✓                                                                                                                                                                                                            100% ██████████
---------------------------------------------------------------------------------------------------------------------- snapshot report summary -----------------------------------------------------------------------------------------------------------------------
56 snapshots passed.

Results (0.59s):
      16 passed

(venv) ➜ ~/Source/home-assistant-core (add_gpu_sensors_to_glances) ✔ ruff format homeassistant tests
11675 files left unchanged

[fhoekstra]

Thanks @joostlek for spotting those issues. I removed and regenerated the snapshots and it turned out to be necessary.

[joostlek]

the 'icon': 'mdi:fan', in the state attributes gave it away as they aren't included anymore with the icon translations :)

[wittypluck]

Nice ! The Disk I/O sensors got merged this morning, I hope the network sensors will also get approved which will make a nice improvement to Glances for 2024.5 !