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

Using uint8 internally to get better performance #1067

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

Conversation

evan361425
Copy link

@evan361425 evan361425 commented Dec 1, 2024

Detailed discussion in #1010

Tested on Android system, but not iOS.

@chipweinberger
Copy link
Owner

chipweinberger commented Dec 1, 2024

"buildPerformanceButton"

what does this screen look like?

please put this in a seperate PR

but realistically the data must be configurable, as most devices dont have a characteristic that just "ignores" data

im not sure ill merge it.

var rawServiceUuids = json['service_uuids'] ?? [];
Map<Object?, Object?> rawManufacturerData = json['manufacturer_data'] ?? <int, List<int>>{};
Map<Object?, Object?> rawServiceData = json['service_data'] ?? <int, List<int>>{};
List<Object?> rawServiceUuids = json['service_uuids'] ?? <String>[];
Copy link
Owner

Choose a reason for hiding this comment

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

please use var. Or give it a proper type. not Object.

Copy link
Author

Choose a reason for hiding this comment

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

rollback as before

@@ -746,7 +746,7 @@ class ServiceDataFilter {
// The 'mask' must have the same length as 'data'.
List<int> mask;

ServiceDataFilter(this.service, {this.data = const [], this.mask = const []});
ServiceDataFilter(this.service, {required this.data, required this.mask});
Copy link
Owner

Choose a reason for hiding this comment

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

this is a breaking change. please undo it.

Copy link
Author

Choose a reason for hiding this comment

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

👌

@@ -726,7 +726,7 @@ class MsdFilter {
/// The 'mask' must have the same length as 'data'.
List<int> mask;

MsdFilter(this.manufacturerId, {this.data = const [], this.mask = const []});
MsdFilter(this.manufacturerId, {required this.data, required this.mask});
Copy link
Owner

Choose a reason for hiding this comment

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

this is a breaking change. please undo it.

Copy link
Author

Choose a reason for hiding this comment

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

👌

@chipweinberger
Copy link
Owner

chipweinberger commented Dec 1, 2024

  1. have you tested it an iOS
  2. please improve the "log" function so that we still log data in the same format as before (unformatted hex string)

@evan361425
Copy link
Author

  • d1df6df rollback breaking changes and perf screen
  • 49a808b log hex when getting uint8list

@evan361425
Copy link
Author

help needed for iOS device, can you help me test it?

@chipweinberger
Copy link
Owner

thanks for the update. I can't test it right now, but maybe another day.

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