refactor: introduce BaseTag to simplify custom tag implementations#7
Merged
Merged
Conversation
Reduce boilerplate and fix drift between the documented capability-based philosophy and what the code actually requires. - Add exported nfc.BaseTag with safe defaults (no-op Connect/Disconnect, NotSupported write/transceive/lock, false IsWritable/CanMakeReadOnly) so custom tags only implement UID/Type/NumericType/ReadData and override what they support. Refactor remotenfc.Tag to embed it, dropping 7 stub methods. - Add DeviceTransceiver optional interface so a polling device whose Transceive is unsupported can report CanTransceive=false; BuildDeviceCapabilities now detects support instead of always asserting true. - Add nfc.AssertCapabilitiesConsistent test helper to catch capability drift using non-mutating checks only. - Remove dead code: unused pcscBaseTag.connected field and vestigial Card.preloadData (+ its silencer var). - Update extending-nfc-support.md and nfc/README.md to document BaseTag, DeviceTransceiver, the consistency helper, and correct the inaccurate "tags connected on first operation" claim. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPoRipkULTkp8gnF2XSSZk
nedpals
changed the title
Drop comments that merely restate the method name or duplicate the type-level doc (remotenfc.Tag accessors, the inherited-methods note already covered by the struct doc, and an obvious metadata-lookup comment). Keep comments that add real information (why NumericType is 0, ReadData's NDEF-over-raw preference). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPoRipkULTkp8gnF2XSSZk
Revert the over-eager comment removal: the prior commit also deleted comments that existed on master. Those are restored. The only comment this PR removes is the dangling "inherited from nfc.BaseTag" note it had itself added, which merely duplicated the Tag struct doc. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPoRipkULTkp8gnF2XSSZk
Drop the per-method restating comments on BaseTag (the type-level doc already explains the override pattern) and collapse a redundant inline note that duplicated the DeviceTransceiver interface doc. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPoRipkULTkp8gnF2XSSZk
nedpals
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces
nfc.BaseTag, an embeddable struct that provides safe default implementations for optional Tag interface methods, significantly reducing boilerplate when implementing custom NFC tags. It also adds capability validation tooling and updates documentation to reflect the new pattern.Key Changes
New
BaseTagtype (nfc/tag_base_default.go): Provides no-op defaults forConnect(),Disconnect(), and "not supported" errors forWriteData(),Transceive(),IsWritable(),CanMakeReadOnly(), andMakeReadOnly(). Custom tags now only need to implement four required methods:UID(),Type(),NumericType(), andReadData().Capability validation (
nfc/capabilities.go): AddedAssertCapabilitiesConsistent()function to detect drift between declaredCapabilities()and actual method behavior (e.g., claimingCanLock: truebut returning false fromCanMakeReadOnly()). Performs only non-mutating checks safe for unit tests.New
DeviceTransceiverinterface (nfc/capabilities.go): Allows devices to explicitly declare whether they support raw transceive operations, overriding the default assumption that polling devices support it.Updated implementations:
nfc/remotenfc/tag.go: Smartphone tags now embedBaseTaginstead of implementing all methods explicitlynfc/tag_base.go: Removed unusedconnectedfield frompcscBaseTagnfc/card.go: Removed unusedpreloadData()methodComprehensive test coverage (
nfc/tag_base_default_test.go): New tests verifyBaseTagdefaults work correctly and demonstrate that a minimal read-only tag needs only four methods.Documentation updates (
docs/extending-nfc-support.md): Completely restructured the tag implementation guide to emphasize embeddingBaseTag, clarified which methods are required vs. optional, added examples showing the dramatic reduction in boilerplate, and included guidance on keeping capabilities honest.Notable Details
Capabilities()and only override the methods backing those capabilitiesBaseTagis a zero-sized type with no state, making it a pure mixin with no memory overheadAssertCapabilitiesConsistent()is intentionally read-only to be safe for unit tests without requiring hardware accesshttps://claude.ai/code/session_01NPoRipkULTkp8gnF2XSSZk