Skip to content

fix: type *devices/*device_uids as str, not List[str] - #2907

Open
proscar87 wants to merge 1 commit into
postlund:masterfrom
proscar87:fix/output-devices-args-type-annotation
Open

proscar87 wants to merge 1 commit into
postlund:masterfrom
proscar87:fix/output-devices-args-type-annotation

Conversation

@proscar87

Copy link
Copy Markdown

Summary

Closes #2828. For a variadic parameter, the annotation types each individual argument, not the collection they get gathered into at the call site -- *devices: str means "each positional arg is a str", matching how these functions are actually called (set_output_devices(*device_ids) with a list of individual device ID strings unpacked via *) and how they're implemented internally (each element treated as one string, never a list, at every call site in this codebase).

Same bug in four places, all fixed the same way:

  • pyatv/interface.py -- the three abstract declarations (add_output_devices, remove_output_devices, set_output_devices)
  • pyatv/core/facade.py -- the facade relay, which just forwards *devices unchanged
  • pyatv/protocols/mrp/__init__.py -- the MRP protocol implementation
  • pyatv/protocols/mrp/messages.py -- the underlying protobuf message builders (add_output_devices/remove_output_devices/set_output_devices), which iterate device_uids and append each one individually to a repeated protobuf field -- one more instance of the same mistake the issue didn't call out, found while checking every call site

List became an unused import in messages.py after the fix; removed.

Testing

This is a pure static-typing annotation fix with no runtime behavior change, so verification is via mypy, not pytest assertions:

  • Reproduced the exact error from the issue in isolation first: a minimal *devices: List[str] class definition, called the way the issue describes, gives mypy's exact reported message (incompatible type "*list[str]"; expected "list[str]"). The same pattern with *devices: str type-checks clean.
  • Ran mypy (the project's own pyatv --ignore-missing-imports --follow-imports=skip invocation from chickn.yaml) before and after on the full pyatv package: 14 pre-existing, unrelated errors in both runs (in dns.py, http.py, metadata.py, pairing.py, scan.py, one line of mrp/__init__.py unrelated to output devices, and __init__.py) -- none of them ever mention output_devices, and the count and file list are identical before/after this change.
  • Full test suite: 1245 passed, 1 pre-existing unrelated failure (test_scan_with_zeroconf_complete_and_device_info_specific_host_matching, an IndexError that reproduces identically on unmodified main -- confirmed via git stash before/after), 6 skipped.
  • black --check, flake8, and pylint (10.00/10) all clean on the four changed files.

🤖 Generated with Claude Code

For a variadic parameter, the annotation types each individual
argument, not the collection they're gathered into -- *devices: str
means "each positional arg is a str", matching how add_output_devices/
remove_output_devices/set_output_devices are actually called (with
individual device ID strings unpacked via *) and how they're
implemented (each element treated as one string, never a list).

Same bug in four places: the abstract declarations in interface.py,
the facade relay in core/facade.py, the MRP protocol implementation in
protocols/mrp/__init__.py, and the underlying protobuf message
builders in protocols/mrp/messages.py.

Fixes postlund#2828

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

type mismatch reported for audio.set_output_devices(self, *devices: List[str])

1 participant