Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the API by introducing type-safe protocols for card identifiers and RF configurations, while standardizing memory access methods across card types. The changes make the API more consistent and easier to use correctly.
Changes:
- Introduced
UniqueIdprotocol classes (Iso14443AUniqueId,Iso15693UniqueId) to replace raw bytes for card identifiers - Added
TxProtocolandRxProtocolenums to replace magic numbers in RF configuration - Standardized memory read/write methods across card types to use byte offsets instead of page/block numbers
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pn5180_tagomatic/cards.py | New file defining UniqueId protocol and card-specific implementations |
| src/pn5180_tagomatic/constants.py | Added TxProtocol and RxProtocol enums, updated MemoryWriteError with offset parameter |
| src/pn5180_tagomatic/session.py | Updated methods to use new UniqueId types instead of raw bytes |
| src/pn5180_tagomatic/iso14443a.py | Refactored to use Iso14443AUniqueId, standardized memory methods to use byte offsets |
| src/pn5180_tagomatic/iso15693.py | Refactored to use Iso15693UniqueId, standardized memory methods to use byte offsets |
| src/pn5180_tagomatic/proxy.py | Updated to use Iso15693UniqueId and new protocol enums |
| src/pn5180_tagomatic/pn5180.py | Updated start_session to use TxProtocol and RxProtocol |
| src/pn5180_tagomatic/init.py | Exported new TxProtocol and RxProtocol enums |
| examples/*.py | Updated examples to use new protocol enums and card identifier types |
| Makefile | Added examples directory to formatting targets |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bofh69
force-pushed
the
improve-api
branch
4 times, most recently
from
e5c2eb1 to
60aed6a
Compare
|
@copilot Fix the failed unit test. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/pn5180_tagomatic/iso15693.py
Outdated
| def _fetch_sys_info_if_needed(self) -> None: | ||
| if self._block_size < 0: |
There was a problem hiding this comment.
The method name _fetch_sys_info_if_needed is ambiguous about what triggers the fetch. Consider renaming to _ensure_sys_info_loaded or _lazy_load_sys_info to better convey that it initializes card metadata on first access.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Also adds properties for AFI, DSFID and IC reference for ISO 15639 cards
No description provided.