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.

Sign up for GitHub

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

Jump to bottom

Feature: pass raw bytes from native, parse using dart-opendroneid #21

Merged
merged 11 commits into from
Sep 12, 2023
Merged

Feature: pass raw bytes from native, parse using dart-opendroneid #21

merged 11 commits into from
Sep 12, 2023

Conversation

matejglejtek
Copy link
Contributor

@matejglejtek matejglejtek commented Aug 27, 2023 •
edited

Remove parsing logic from native parts of the app, remove all models and enums representing messages and values in them. Rename MessagePack to MessageContainer to avoid clash with dart-opendroneid's MessagePack.

Add ODIDPayload class that is sent from native to dart. It contains raw data and metadata such as source and mac address. In dart code, parse the data using dart-opendroneid into messages and save them to MessageContainer.

To test this,edit path in pubspec.yaml to point to your cloned dart-opendroneid repository.
Clone DroneScanner branch use-updated-odid and run it with this version of flutter-opendroneid.

TODO

  • test all possible sources - bt5, wi-fi - I will test wi-fi ODID later this week with help of David
  • make sure duplicate messages filtered out
  • refactor kotlin scanner classes
  • test MessagePacks, make sure they transform to MessageContainer correctly - error found in Dart-opendroneid MessagePack parsing - PR1.
  • wait for release of dart-opendroneid and change dependency
  • update documentation

@matejglejtek matejglejtek force-pushed the feature/DT-2604-pass-raw-bytes-from-native branch 4 times, most recently from ada49b6 to d825157 Compare August 30, 2023 13:34
@matejglejtek matejglejtek force-pushed the feature/DT-2604-pass-raw-bytes-from-native branch 2 times, most recently from 5f74ecd to 2283f42 Compare August 31, 2023 13:59
@matejglejtek
Copy link
Contributor Author

Update:

I tried scanning for data from all sources including simulated Wi-FI scan using ESP and achieved the same results as with the original version. Parsing of all message types including MessagePack works. I found few issues with dart-odid and immediately solved them (PR1)

Waiting for release of dart-odid.

@marianhlavac
Copy link
Member

Pls don't forget to assign reviewers, this was the PRs gets easily missed.

@marianhlavac
Copy link
Member

dart_opendroneid is released: https://pub.dev/packages/dart_opendroneid/versions/0.1.0

@matejglejtek matejglejtek force-pushed the feature/DT-2604-pass-raw-bytes-from-native branch from 2283f42 to 2288dd1 Compare September 1, 2023 15:54
@matejglejtek
Copy link
Contributor Author

I used the released version of dart_opendroneid. But since PR1 was not resolved yet, MessagePacks wont be parsed correctly and BT data wont be parsed on some phones.

@matejglejtek matejglejtek force-pushed the feature/DT-2604-pass-raw-bytes-from-native branch 2 times, most recently from 04a62f9 to d122320 Compare September 4, 2023 10:15
@matejglejtek matejglejtek force-pushed the feature/DT-2604-pass-raw-bytes-from-native branch from d122320 to cdacbaf Compare September 4, 2023 10:36
albertmoravec
albertmoravec approved these changes Sep 8, 2023
View reviewed changes
Copy link
Contributor

@albertmoravec albertmoravec left a comment •
edited

Choose a reason for hiding this comment

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

I gave some comments on very minor issues, but I'll let you decide, since I don't have any real objections that would prevent merging.

Also do you think that maybe we could move some of the conversion and comparison code from compare_extensions.dart and conversions.dart to dart-opendroneid?

lib/models/message_container.dart Show resolved Hide resolved
lib/models/message_container.dart Outdated Show resolved Hide resolved
android/src/main/kotlin/cz/dronetag/flutter_opendroneid/scanner/ODIDScanner.kt Outdated Show resolved Hide resolved
android/src/main/kotlin/cz/dronetag/flutter_opendroneid/scanner/ODIDScanner.kt Outdated Show resolved Hide resolved
@matejglejtek
Copy link
Contributor Author

I used the getters in MessageContainer, renamed method getPayload to buildPayload. Maybe we could create new task to resolve situation with MessagePack vs MessageContainer and moving conversions to dart-opendroneid?

@matejglejtek matejglejtek force-pushed the feature/DT-2604-pass-raw-bytes-from-native branch from ac9dbf0 to 790ff7d Compare September 11, 2023 11:55
@matejglejtek
refactor: rename method to build payload, refactor MessageContainer
cb59744 
use getters in message container, add comments

DT-2604
@matejglejtek matejglejtek force-pushed the feature/DT-2604-pass-raw-bytes-from-native branch from 790ff7d to cb59744 Compare September 11, 2023 12:07
@marianhlavac
Copy link
Member

Maybe we could create new task to resolve situation with MessagePack vs MessageContainer and moving conversions to dart-opendroneid?

Yeah, this will be ultimately resolved by also joining the structures that are used by the upcoming 'DRI Receiver' product. We don't have to resolve it right now.

marianhlavac
marianhlavac approved these changes Sep 12, 2023
View reviewed changes
Copy link
Member

@marianhlavac marianhlavac left a comment

Choose a reason for hiding this comment

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

Looks great!

Tested on my iPhone X, data seems also ok, but more thorough testing will be required in the future.

We will follow up with the structures for the metadata (the new container class you've created). There is an opportunity to use unified structures, as 'DRI Receiver' and potentially mobile app could use them too.

lib/utils/conversions.dart
Copy link
Member

Choose a reason for hiding this comment

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

I think that this conversions could be a part of dart_opendroneid in the future.

@marianhlavac marianhlavac merged commit 87f5481 into master Sep 12, 2023
1 check passed
@marianhlavac marianhlavac deleted the feature/DT-2604-pass-raw-bytes-from-native branch September 12, 2023 09:25
github-actions bot pushed a commit that referenced this pull request Sep 12, 2023
@semantic-release-bot
chore(release): 0.14.0 [skip ci]
4c90c1f 
# [0.14.0](v0.13.0...v0.14.0) (2023-09-12)

### Bug Fixes

* update container with source of current message update ([c477388](c477388))

### Features

* add conversions of message values to strings ([7b43b50](7b43b50))
* put back duplicate messages filtering ([f1d1001](f1d1001))
* remove message parsing, use dart-odid ([c3dda27](c3dda27))
* shorted BLE advertisements to max 31 bytes ([8da4ea2](8da4ea2))
* Use dart-opendroneid for parsing & pass raw bytes from native (DT-2604) ([#21](#21)) ([87f5481](87f5481))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marianhlavac marianhlavac marianhlavac approved these changes

@albertmoravec albertmoravec albertmoravec approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
3 participants
@matejglejtek @marianhlavac @albertmoravec