-
Notifications
You must be signed in to change notification settings - Fork 493
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
[Android] distinguish read and notification for future completion #1118
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -459,6 +459,12 @@ class BmReadCharacteristicRequest { | |
} | ||
} | ||
|
||
enum BmReceiveEventEnum { | ||
unknown, // 0 | ||
read, // 1 | ||
notify, // 2 | ||
} | ||
|
||
class BmCharacteristicData { | ||
final DeviceIdentifier remoteId; | ||
final Guid serviceUuid; | ||
|
@@ -468,6 +474,7 @@ class BmCharacteristicData { | |
final bool success; | ||
final int errorCode; | ||
final String errorString; | ||
final BmReceiveEventEnum receiveEvent; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets make this nullable |
||
|
||
|
||
BmCharacteristicData({ | ||
|
@@ -479,6 +486,7 @@ class BmCharacteristicData { | |
required this.success, | ||
required this.errorCode, | ||
required this.errorString, | ||
this.receiveEvent = BmReceiveEventEnum.unknown, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets make it required, for consistency There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Leaving it optional means that we need not make this a breaking change and would be better if possible. We could default to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. breaking is fine tho. I still dont really understand why we wouldn't want to break. now is the perfect time to break since we have no other users of this interface. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
}); | ||
|
||
factory BmCharacteristicData.fromMap(Map<dynamic, dynamic> json) { | ||
|
@@ -491,6 +499,7 @@ class BmCharacteristicData { | |
success: json['success'] != 0, | ||
errorCode: json['error_code'], | ||
errorString: json['error_string'], | ||
receiveEvent: BmReceiveEventEnum.values[json['receive_event']], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets make an explicit 'parse' helper function |
||
); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets just have this:
and then make it nullable, and have a
BmReceiveType? _parseReceiveTypeFromInteger
function