Skip to content

Conversation

@ralequi
Copy link
Collaborator

@ralequi ralequi commented Oct 1, 2025

Summary

  • reset Device defaults for SMART capability, enablement, and SSD status instead of special-casing NVMe-derived interfaces
  • rely on the canonical interface returned by _classify during update() to flag NVMe devices (including ASMedia bridges) as SMART-capable SSDs
  • drop the now-unnecessary _interface shortcut that previously toggled the NVMe defaults during interface probing

Testing

  • pip install .
  • pytest

https://chatgpt.com/codex/tasks/task_e_68dd1e61b4e0832fa014ceacb486f993

@ralequi ralequi self-assigned this Oct 1, 2025
@ralequi ralequi requested a review from Copilot October 1, 2025 15:57
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors NVMe device detection and SMART capability assignment to use the canonical interface classification instead of early interface-specific defaults. The change ensures that NVMe devices accessed through bridge interfaces like ASMedia are properly flagged as SMART-capable SSDs.

  • Removes hardcoded NVMe defaults during device initialization and interface probing
  • Adds canonical interface mapping for ASMedia bridge (sntasmedianvme)
  • Sets SMART flags based on canonical interface during the update process

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pySMART/device.py Updated device initialization defaults and classification logic
pySMART/utils.py Added sntasmedia interface mapping
tests/dataset/singletests/nvme_usb_asmedia_issue_97/device.json Test data for ASMedia NVMe device
tests/dataset/singletests/nvme_usb_asmedia_issue_97/-d_sntasmedia--all__dev_sda Raw smartctl output for test case
tests/dataset/listingtests/linux_usb_nvme_asmedia_issue_97/device.json Test data for device listing
tests/dataset/listingtests/linux_usb_nvme_asmedia_issue_97/-d_sntasmedia--all__dev_sda Raw smartctl output for listing test
tests/dataset/listingtests/linux_usb_nvme_asmedia_issue_97/_--scan-open Device scan output

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@ralequi ralequi linked an issue Oct 1, 2025 that may be closed by this pull request
@ralequi ralequi changed the base branch from master to develop October 1, 2025 16:02
@ralequi
Copy link
Collaborator Author

ralequi commented Oct 1, 2025

@codex please, mention #97 and its creator on the futute release notes as I've been done in other cases

@chatgpt-codex-connector
Copy link

Summary

Testing

  • ⚠️ Not run (not requested)

View task →

@ralequi ralequi requested a review from Copilot October 1, 2025 17:08
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +1017 to +1021
canonical_interface = self.dev_interface
if canonical_interface == 'nvme':
self.smart_capable = True
self.smart_enabled = True
self.is_ssd = True
Copy link

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable name canonical_interface is misleading since it's assigned self.dev_interface but the comment and PR description suggest it should use the result from _classify(). Either use self._classify() or rename the variable to reflect it's using dev_interface.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot could you fix this for me?

@ralequi
Copy link
Collaborator Author

ralequi commented Oct 1, 2025

@coplilot please, mention #97 and its creator on the future release notes (1.4.2) as I've been done in other cases

@ralequi ralequi merged commit 78bfc7c into develop Oct 1, 2025
1 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ValueError: Unknown interface: sntasmedia specified for /dev/sda

2 participants