Skip to content

BLE communication refactoring#7

Open
monday8am wants to merge 5 commits intomasterfrom
descriptor-fix
Open

BLE communication refactoring#7
monday8am wants to merge 5 commits intomasterfrom
descriptor-fix

Conversation

@monday8am
Copy link
Copy Markdown
Contributor

Hint for reviewer:

This code works right now. The next topic is to decide where to include it (as part of our lib or as a demo implementation)

@monday8am monday8am requested a review from sergkh June 6, 2017 12:38
package com.fidesmo.ble.client.apdu;


public interface CustomPacketDefragmenter {
Copy link
Copy Markdown
Member

@sergkh sergkh Jun 6, 2017

Choose a reason for hiding this comment

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

Maybe we can think on how to merge this CustomPacketDefragmenter with original Packet fragmenter/defragmenter interfaces? Some of those methods can be handy there too.

But I have some notes about the interface:

  • Method add is not informative, looking at the interface you can't say what it is doing and how it is different from the append. Maybe we can rename both to something like addPacket and addBuffer (or nay other variant)?
  • Interface is not consistent: you have Java Bean style getBuffer and simplified complete (which in turn has to be completed maybe?) and empty. IHMO it would be good to choose one naming scheme or append isCompleted or remove get what is better to you.
  • Original intent was to not use clear and simply recreate the builder, why do you want to reuse it?
  • Github complains about not having newline at the end

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.

2 participants