docs: clarify write operations in Javadoc#11
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates public API documentation to better describe the expected contract for write operations (especially for technologies with unreliable write acknowledgments), and bumps the snapshot version accordingly.
Changes:
- Clarify Javadoc contracts for
prepareSt25WriteSystemBlockandprepareWriteBlocksaround “expected final state” and automatic verification reads. - Bump project version to
1.2.1-SNAPSHOT. - Add an entry to the changelog describing the Javadoc clarification.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/main/java/org/eclipse/keypop/storagecard/transaction/StorageCardTransactionManager.java |
Updates Javadoc for write methods to clarify write/verification expectations. |
gradle.properties |
Increments the snapshot version to the next patch (1.2.1-SNAPSHOT). |
CHANGELOG.md |
Records the Javadoc clarification under [unreleased]. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * verify the actual content stored on the card. | ||
| * <p><strong>Important:</strong> It is the user's responsibility to ensure that the write | ||
| * operations are coherent with the target card's technology (e.g., OTP bits). The data provided | ||
| * must represent the <b>expected final state</b> of the system block after the write operation. * |
There was a problem hiding this comment.
There’s a stray * at the end of the Javadoc sentence (...after the write operation. *). This will render oddly in generated Javadoc; remove the trailing asterisk (or escape it if it’s meant to be literal).
| * must represent the <b>expected final state</b> of the system block after the write operation. * | |
| * must represent the <b>expected final state</b> of the system block after the write operation. |
| * <p><strong>Important:</strong> It is the user's responsibility to ensure that the write | ||
| * operations are coherent with the target card's technology (e.g., OTP bits, counters). The data | ||
| * provided must represent the <b>expected final state</b> of the blocks after the write | ||
| * operation. * |
There was a problem hiding this comment.
There’s a stray * at the end of the Javadoc sentence (...after the write operation. *). This will render oddly in generated Javadoc; remove the trailing asterisk (or escape it if it’s meant to be literal).
| * operation. * | |
| * operation. |
| * <p>ST25/SRT512 cards do not provide reliable status codes to confirm successful write | ||
| * operations. Consequently, the library performs an automatic verification read. This check will | ||
| * fail if the physical state of the system block after the write does not exactly match the | ||
| * provided data. |
There was a problem hiding this comment.
The updated contract says the library performs an automatic verification read after some writes, but the interface-level note above still states that for write commands the StorageCard memory image is not updated and that applications should perform explicit reads after writes. Please reconcile these docs by clarifying whether the verification read (a) updates the StorageCard memory image or (b) is only an internal comparison. Also consider stating what the caller should expect during processCommands() when verification fails (e.g., whether it throws SCCardCommunicationException, per that exception’s Javadoc), so users know how to handle the failure.
| * performs an automatic verification read. This check will fail if the physical state of the card | ||
| * after the write does not exactly match the provided data. For cards providing reliable | ||
| * acknowledgment, no additional read is performed. |
There was a problem hiding this comment.
This Javadoc describes an automatic verification read for cards without reliable write status codes, which conflicts with the interface-level note stating that applications should perform explicit reads after writes and that the StorageCard memory image is not updated for write commands. Please align these two pieces of documentation and clarify whether the verification read updates the StorageCard memory image or is only used for internal comparison during processCommands().
| * performs an automatic verification read. This check will fail if the physical state of the card | |
| * after the write does not exactly match the provided data. For cards providing reliable | |
| * acknowledgment, no additional read is performed. | |
| * performs an automatic verification read during {@link #processCommands(ChannelControl)} for | |
| * internal comparison with the provided data. This check will fail if the physical state of the | |
| * card after the write does not exactly match the expected final state. This verification read | |
| * does <strong>not</strong> update the {@link StorageCard} memory image; applications must | |
| * prepare an explicit read command after the write if they need to refresh the memory image. For | |
| * cards providing reliable acknowledgment, no additional verification read is performed. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * <p><strong>For write commands:</strong> The {@link StorageCard} memory image is updated with the | ||
| * written data once the write is confirmed successful. For card technologies that do not provide | ||
| * reliable status codes (e.g. ST25/SRT512), confirmation is obtained via an automatic verification | ||
| * read; the memory image is updated only if this verification passes. |
There was a problem hiding this comment.
The class-level Javadoc now states that the StorageCard memory image is updated for write commands once the write is confirmed, but the deprecated prepareWriteSystemBlock(byte[]) Javadoc below still says the memory image is not automatically updated (lines 81-85). Please reconcile these contracts (e.g., clarify that only certain write operations update the memory image, or update the deprecated method’s documentation accordingly) to avoid conflicting guidance for API consumers.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * provided must represent the <b>expected final state</b> of the blocks after the write | ||
| * operation. | ||
| * | ||
| * <p>For cards that do not provide reliable status codes (e.g., SRT512/ST25), the library |
There was a problem hiding this comment.
Card family naming is inconsistent in this Javadoc (ST25/SRT512 vs SRT512/ST25). Please standardize the order across the file to keep the public API documentation searchable and unambiguous (ideally match ProductType.ST25_SRT512).
| * <p>For cards that do not provide reliable status codes (e.g., SRT512/ST25), the library | |
| * <p>For cards that do not provide reliable status codes (e.g., ST25/SRT512), the library |
| ### Changed | ||
| - **Javadoc**: clarified write contract for `prepareWriteBlocks` and `prepareSt25WriteSystemBlock`. |
There was a problem hiding this comment.
The changelog entry says the Javadoc clarification is limited to prepareWriteBlocks and prepareSt25WriteSystemBlock, but this PR also changes the higher-level processCommands() write contract (memory image update semantics) and removes text from the deprecated prepareWriteSystemBlock docs. Please adjust this entry to reflect the full scope of the documentation changes (or narrow the code changes to match the entry).
No description provided.