Display QR code's finder pattern as feedback that QR code has been detected

lcasburn commented 10 years ago

Ready for code-review on branch ticket-66.

creswick commented 10 years ago

A few comments -- I'm happy to see if we can come up with some simplifications for ReceiveFragment (and work on ideas for Receive, too).

ReceiveFragment

Please don't use static fields (camera and cameraOverlay). They are global variables, even when they are private they tend to cause issues down the line that are difficult to debug. We may want to revisit the design of the inner classes to make it easier to connect the components, particularly since ReceiveFragment has grown substantially since it was created initially.

QRPointsFoundView

onDraw: is it possible to save and re-use the transformation matrix across draw calls?

Receive

saveMessageAndUpdateProgress: The control flow is pretty difficult to follow here because it relies so heavily on the exceptions thrown from the other saveMessageAndUpdateProgress. I think it would be easier to follow if the failures were more obvious. The private version of the method is only ever called from this for loop: could we rename it and make the return value also encode whether it was successfull? (maybe even returning null instead of an exception, or creating a wrapper type for the error state.)

lcasburn commented 10 years ago

ReceiveFragment

Implemented changes to camera and cameraOverlay (commits: 9af003d74f00f6720e5636a915ca1740e2971cf0 and 93546b5a916fae58e00c3c4735f7fd55205913f0 respectively).

We may want to revisit the design of the inner classes to make it easier to connect the components, particularly since ReceiveFragment has grown substantially since it was created initially.

Holding off on changing design of inner classes until some of the other critical issues in milestone 1.0 are completed.

QRPointsFoundView

Create transformation matrix whenever the camera parameters are set and reuse it onDraw. See 8c61fa16768319bac92ea58d0b664d468e4a194d.

Receive

Improving control flow of saveMessageAndUpdateProgress changes are in 52d0d12faebfa5f9050aa980c84f9f77536afb9f.

PartialMessage

Fixes error in getRawData that was thrown when QR Result had more BYTE_SEGMENTS than expected. This was triggered by trying to extract data from QR Result with type URI. I modified this function to extract the raw byte data only when it has TEXT type and also make sure to append all BYTE_SEGMENTS together before returning.

creswick commented 10 years ago

Looks good to me!

lcasburn commented 10 years ago

Branch ticket-66 is now merged with master.

creswick / StreamingQR