Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Version 0.2: simplification and clarification #16

Closed
wants to merge 2 commits into from
Closed

Conversation

martinpaljak
Copy link
Member

Address #14: Only a single APDU at a time can be exchanged over BLE (as a SE can only process a single command at a time) and the responsibility of GET RESPONSE handling is clarified
Address #7: one level of wrapping and related packet overhead is removed by addressing #14
Address #13: recommend re-sending the notification if response is not read in a timely fashion
Address #11: have the response notify include an error code

Further reduction of the overhead in payload headers should be considered.

@martinpaljak martinpaljak requested review from miguelcardo and sergkh and removed request for miguelcardo December 12, 2018 11:00
Copy link
Member

@sergkh sergkh left a comment

Choose a reason for hiding this comment

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

Looks good to me, although I would advice to minimize packet headers size, as now packets will be smaller

![APDU-over-BLE](https://www.websequencediagrams.com/cgi-bin/cdraw?lz=dGl0bGUgQVBEVS1vdmVyLUJMRQoKcGFydGljaXBhbnQgIk1vYmlsZSBBcHAiIGFzIENsaWVudAAVDkJMRSBQZXJpcGhlcmFsACIFU2VydmVyADwOU2VjdXJlIEVsZW1lbnQAIQZFCiAKYWx0IEJsdWV0b290aCBjb25uZWN0aW9uCgA_Bi0-AG4GOiBBZHZlcnRpc2Ugc2VydmljZQoAgQgGLT4AaQY6IEMANwYAMggAMAtDSwAcEUJvbmQAFBZlbmQKbG9vcCBNdWx0aXAAghsHcwoADwZBUERVIHRvIACBaQ4gMS4uTgCBABFXcml0ZTogIgAvBUNvbW1hbmQiIHdpdGggZnJhZ21lbnQgTgBrGm5vdGUgcmlnaHQgb2YAgk4HOgB0BXAAbwpNQVkgcG93ZXIgdXAgU0UKAIJMBQCBHQgAgmcOAIJKCVNFOgCBBwhBUERVClNFAII7ClJlc3BvbnNlABMGAIJCCAAuBTB4NjFYWCAvIDB4NkNYWCBoYW5kbGluZwpTRQAeBQAuFGVuZAoAgzkSUERVAGAJIFJlYWR5IE5vdGlmaWNhAIN3BQCCXQ4AhHAGAIJNFlJlYWQAgloIAIEzCCIAg3ASAIFKDACCcAwAgRoFYWx0IEluIGNhc2Ugb2YgZXJyb3IAhDMSAIEcBXkAgzgGAB4FIGNvZGUAhCkFZW5kAINjGUNvbnZlcnMAgVMFIEZpbmlzaGVkIgCDLDBkb3duIFNFAIVFFgo&s=rose)


```
Copy link
Member

Choose a reason for hiding this comment

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

I would move that to a separate file

Copy link
Member Author

Choose a reason for hiding this comment

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

Until finalization, it is easier to keep it as such "editable link". For finalization, the images folder shall be cleaned.

Copy link
Member

Choose a reason for hiding this comment

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

Agree with @sergkh

Copy link
Member

@miguelcardo miguelcardo left a comment

Choose a reason for hiding this comment

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

Many thanks for bringing it up to date, syncying it with the work done.
Please have a look at my comments.


The sequence of APDUs is then divided into packets that fit into the maximum length for this characteristic, following the structure [below](#packet).
#### Characteristic: APDU Response
Used by the Client to retrieve the Response APDU fragments corresponding to the processing of a previous Command APDU. The Client will read this characteristic only after receiving *APDU Response Ready* notification. Response APDU fragments will be encoded in the same structure as command APDUs in the figure below.

#### Characteristic: Conversation Finished
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be called "Close Communication"? Conversation Finished was removed from the table

![APDU-over-BLE](https://www.websequencediagrams.com/cgi-bin/cdraw?lz=dGl0bGUgQVBEVS1vdmVyLUJMRQoKcGFydGljaXBhbnQgIk1vYmlsZSBBcHAiIGFzIENsaWVudAAVDkJMRSBQZXJpcGhlcmFsACIFU2VydmVyADwOU2VjdXJlIEVsZW1lbnQAIQZFCiAKYWx0IEJsdWV0b290aCBjb25uZWN0aW9uCgA_Bi0-AG4GOiBBZHZlcnRpc2Ugc2VydmljZQoAgQgGLT4AaQY6IEMANwYAMggAMAtDSwAcEUJvbmQAFBZlbmQKbG9vcCBNdWx0aXAAghsHcwoADwZBUERVIHRvIACBaQ4gMS4uTgCBABFXcml0ZTogIgAvBUNvbW1hbmQiIHdpdGggZnJhZ21lbnQgTgBrGm5vdGUgcmlnaHQgb2YAgk4HOgB0BXAAbwpNQVkgcG93ZXIgdXAgU0UKAIJMBQCBHQgAgmcOAIJKCVNFOgCBBwhBUERVClNFAII7ClJlc3BvbnNlABMGAIJCCAAuBTB4NjFYWCAvIDB4NkNYWCBoYW5kbGluZwpTRQAeBQAuFGVuZAoAgzkSUERVAGAJIFJlYWR5IE5vdGlmaWNhAIN3BQCCXQ4AhHAGAIJNFlJlYWQAgloIAIEzCCIAg3ASAIFKDACCcAwAgRoFYWx0IEluIGNhc2Ugb2YgZXJyb3IAhDMSAIEcBXkAgzgGAB4FIGNvZGUAhCkFZW5kAINjGUNvbnZlcnMAgVMFIEZpbmlzaGVkIgCDLDBkb3duIFNFAIVFFgo&s=rose)


```
Copy link
Member

Choose a reason for hiding this comment

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

Agree with @sergkh


### Responses
When sending a Command APDU, the Client sends the different fragments in order, waiting for the ACK from the Server (sent by lower layers of the BLE stack).
Copy link
Member

Choose a reason for hiding this comment

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

The "Server" is not identified in the diagram. Wouldn't it better to use the same actor names as in the sequence?

@martinpaljak
Copy link
Member Author

stale

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.

3 participants