Skip to content
This repository has been archived by the owner on Jan 27, 2024. It is now read-only.

readUntilBytes function add #2

Closed
wants to merge 38 commits into from
Closed

readUntilBytes function add #2

wants to merge 38 commits into from

Conversation

AleyRobotics
Copy link
Contributor

func readUntilBytes(stopBytes: [UInt8], maxBytes: Int) throws -> [UInt8]
reads bytes until bytes was found or maxBytes count received

@yeokm1
Copy link
Owner

yeokm1 commented Jun 28, 2017

Can you update your existing codebase to include my changes after I merged your previous pull request #1 to avoid merge conflicts? Also, do update the documentation in README.md. I did it for you the previous time but please try to do it this time.

new function description add
@AleyRobotics
Copy link
Contributor Author

i have updated README.md
Can you give me instructions how to update my existing codebase to include yours changes after you merged my previous pull request #1?

i have add one function, other code was not changed. Why i have got merge conflicts? 0_0

`public func readUntilBytes(stopBytes: [UInt8], maxBytes: Int) throws -> [UInt8] {
let buffer = UnsafeMutablePointer.allocate(capacity: 1)
var data = UInt8
defer {
buffer.deallocate(capacity: 1)
}

    while true {
        let byteRead = UInt8(try readBytes(into: buffer, size: 1))
        
        if byteRead > 0 {
            data.append(buffer[0])
            if data.count >= stopBytes.count {
                for index in (0..<stopBytes.count).reversed() {
                    var isStopFound = 0
                    if stopBytes[index] == data[data.count - index - 1] {
                        isStopFound = isStopFound + 1
                    }
                    if isStopFound == stopBytes.count {
                        return data
                    }
                }
            }
            
            if data.count >= maxBytes {
                return data
            }
        }
    }
}

`

@AleyRobotics
Copy link
Contributor Author

please check merge.
Did i all done well?
Sorry, English is not my first one language.

@yeokm1
Copy link
Owner

yeokm1 commented Jul 1, 2017

I have some changes I suggest you should make before I merge. I trust your code logic is correct.

  1. Can these let buffer = UnsafeMutablePointer<UInt8>.allocate(capacity: 1) in line 483 and swift defer { buffer.deallocate(capacity: 1)} in line 485 to 487 be placed together?

Move var data = [UInt8]() to either above or below them

var data = [UInt8]() should not have any issues but it is good practice to put the defer immediately after the allocating code.

  1. In line 490, why do you use the readBytes(into: buffer, size: 1)) when you have the readByte() function now? Once you use readByte() it saves you the need of allocating the UnsafeMutablePointer in this function. If you do this, you don't need step 1 anymore.

  2. The function is long and comments to how it works should be added.

  3. Follow my readme convention of "this function internally calls X".

@AleyRobotics
Copy link
Contributor Author

AleyRobotics commented Jul 4, 2017

Received byte with value 0x0D becomes 0x0A bug fixed
code optimizations

@yeokm1
Copy link
Owner

yeokm1 commented Jul 10, 2017

Err, I've been reluctant to check you pull request as you keep modifying it. Once you are done, can you squash your commits into one to make review easier?

@AleyRobotics
Copy link
Contributor Author

squash fail. Created new pull request

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants