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

Add filter for and services #24

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

brymez1609
Copy link

No description provided.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Wilson Jaramillo seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Contributor

@jpsoultanis jpsoultanis left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution to RxCBCentral! In general things look good, just please cleanup debugging comments and remove your print statements.

@@ -80,6 +80,10 @@ public protocol RxPeripheral: AnyObject {
/// The data returned will be processed by the `Preprocessor` given when registering
/// for notifications for this `characteristic`, if one was provided.
func notificationData(for characteristic: CBUUID) -> Observable<Data>

/// Validate if service exist
func hasService(service: CBUUID) -> Observable<Bool>
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice addition, can you use modify this interface to use more Swift style naming, like:

func hasService(_ service: CBUUID) -> Observable<Bool>

Comment on lines +192 to +193
print("RFN ",services.count, services)
print("RFN ",services.first?.uuid.uuidString, service.uuidString)
Copy link
Contributor

Choose a reason for hiding this comment

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

Printing is helpful for debugging purposes but we don't want to ship the library with live print statements. Please remove or add these to the logger.

Comment on lines +228 to +239
print("RFN characteristics ",characteristics.count, characteristics)
for c in characteristics {
print(c.uuid.uuidString, characteristic.uuidString)
if(c.uuid.uuidString == characteristic.uuidString){
return (c, error)
}
}
print("not found , what to do, raise an error")
return (characteristics.first,GattError.characteristicNotFound)
// print("RFN ",characteristics.first?.uuid.uuidString , characteristic.uuidString)
// let characteristic = characteristics.first { $0.uuid.uuidString == characteristic.uuidString }
// return (characteristic, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

Cleanup required

Comment on lines 357 to 359
// des.setValue(<#T##value: Any?##Any?#>, forKey: <#T##String#>)
print("BEGIN:SPAKA DESCRIPTOR========>\(des)")
// self.peripheral.setNotifyValue(true, for: characteristic)
Copy link
Contributor

Choose a reason for hiding this comment

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

cleanup required

Comment on lines 366 to 369
if characteristic.value != nil {
let value = characteristic.value?.subdata(in: Range(1...2)).withUnsafeBytes {$0.load(as: UInt8.self)}
print("didUpdateValueFor Characteristic", characteristic, " value:", value ?? "No data 324")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please delete, since we don't want to ship the library with live print statements

Copy link
Contributor

@mustafagunes mustafagunes left a comment

Choose a reason for hiding this comment

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

First, thank you very much for sending a pull request 👏. But some changes need to be made. Please follow the changes I suggest. After that, your pull request will be reviewed 🙂

@@ -80,6 +80,12 @@ public protocol RxPeripheral: AnyObject {
/// The data returned will be processed by the `Preprocessor` given when registering
/// for notifications for this `characteristic`, if one was provided.
func notificationData(for characteristic: CBUUID) -> Observable<Data>

/// Validate if service exist
func hasService(service: CBUUID) -> Observable<Bool>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func hasService(service: CBUUID) -> Observable<Bool>
func hasService(_ service: CBUUID) -> Observable<Bool>

Comment on lines +75 to +79
if (services.count == 0){
return false
} else {
return true
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (services.count == 0){
return false
} else {
return true
}
return services.count == 0 ? false : true

@@ -94,8 +101,10 @@ class RxPeripheralImpl: NSObject, RxPeripheral, CBPeripheralDelegate {
}
.take(1)
.map { (characteristics: [CBCharacteristic], error: Error?) -> (CBCharacteristic?, Error?) in
print("READ \(#line) ",characteristics.first?.uuid.uuidString,characteristic.uuidString)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
print("READ \(#line) ",characteristics.first?.uuid.uuidString,characteristic.uuidString)

// check that given characteristic exists on the peripheral
let characteristic = characteristics.first { $0.uuid.uuidString == characteristic.uuidString }
print("READ after \(#line) ",characteristic)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
print("READ after \(#line) ",characteristic)

let matchingService = services.first { $0.uuid.uuidString == service.uuidString }
return (matchingService, error)
}
.take(1)
.flatMapLatest { (matchingService: CBService?, error: Error?) -> Observable<([CBCharacteristic], Error?)> in
print("MATCHINGSERVICE :",matchingService)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
print("MATCHINGSERVICE :",matchingService)

Comment on lines +490 to +493
if characteristic.value != nil {
let cmd = [UInt8](characteristic.value ?? Data(_:[0x00]))
print("didUpdateValueFor UINT8: ",cmd)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if characteristic.value != nil {
let cmd = [UInt8](characteristic.value ?? Data(_:[0x00]))
print("didUpdateValueFor UINT8: ",cmd)
}

@@ -243,6 +243,10 @@ public class BluetoothDetectorTypeMock: BluetoothDetectorType {
}

public class CBPeripheralTypeMock: CBPeripheralType {
public func discoverDescriptors(for characteristic: CBCharacteristic) {
print("ok")
Copy link
Contributor

Choose a reason for hiding this comment

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

Please arrange here.

Comment on lines +573 to +578
public func hasService(service: CBUUID) -> Observable<Bool>{
return Observable.just(true)
}
public func setIndicateDescriptor(service: CBUUID, characteristic: CBUUID, preprocessor: Preprocessor? = nil) -> Observable<Bool>{
return Observable.just(true)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public func hasService(service: CBUUID) -> Observable<Bool>{
return Observable.just(true)
}
public func setIndicateDescriptor(service: CBUUID, characteristic: CBUUID, preprocessor: Preprocessor? = nil) -> Observable<Bool>{
return Observable.just(true)
}
public func hasService(service: CBUUID) -> Observable<Bool>{
return Observable.just(true)
}
public func setIndicateDescriptor(service: CBUUID, characteristic: CBUUID, preprocessor: Preprocessor? = nil) -> Observable<Bool>{
return Observable.just(true)
}

}

public func setIndicateDescriptor(service: CBUUID, characteristic: CBUUID, preprocessor: Preprocessor? = nil) -> Observable<Bool>{
print("setIndicateDescriptor")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
print("setIndicateDescriptor")

}
.take(1)
.flatMapLatest { (matchingService: CBService?, error: Error?) -> Observable<([CBCharacteristic], Error?)> in
print("MATCHINGSERVICE SID :",matchingService)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
print("MATCHINGSERVICE SID :",matchingService)

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.

None yet

4 participants