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

added utf8 support #14

Closed
wants to merge 3 commits into from
Closed

Conversation

Buggaboo
Copy link
Contributor

@Buggaboo Buggaboo commented Sep 4, 2019

Changes

  • Something changed on the SDK, it no longer infers the type of the fromAddress and allocate methods.
  • I changed the test to add notes with utf-8 strings as bytes in decimal representation

@greenrobot
Copy link
Member

Thanks for your PR - and congrats to the do the first one ever in this project! 🥇

We'll look into it very soon.

lib/src/ffi/cstring.dart Outdated Show resolved Hide resolved
lib/src/box.dart Outdated
@@ -125,25 +125,25 @@ class Box<T> {
}

// transform flatbuffers byte array into memory area for C, with a length of a multiple of four
Pointer<Uint8> bufferPtr = allocate(count: ((buffer.length + 3.0) / 4.0).toInt() * 4);
Pointer<Uint8> bufferPtr = Pointer<Uint8>.allocate(count: ((buffer.length + 3.0) / 4.0).toInt() * 4);
Copy link
Member

Choose a reason for hiding this comment

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

I'm getting compile errors with those:

lib/src/common.dart:8:50: Error: Method not found: 'Pointer.allocate'.
        Pointer<Int32> majorPtr = Pointer<Int32>.allocate(), minorPtr = Pointer<Int32>.allocate(), patchPtr = Pointer<Int32>.allocate();
                                                 ^^^^^^^^

What's your Dart version?

$ dart --version
Dart VM version: 2.4.1

Maybe we have to increase the Dart version requirement?

Copy link
Contributor

@nalenz-objectbox nalenz-objectbox Sep 5, 2019

Choose a reason for hiding this comment

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

I can confirm @greenrobot's error. If you look at the Dart FFI source file at /usr/lib/dart/lib/ffi/ffi.dart on Linux, you can see that the allocate function is not a static member or constructor of the Pointer class, but just outside of it.

Yet, the FFI SQLite example uses Pointer.allocate as well, despite the underlying source file clearly being different.

Additionally, the master branch of the Dart SDK contains Pointer.allocate in the correct form, see here. This means that either the Dart PPA is deprecated (which is unlikely, as dart --version correctly says 2.4.1 on my machine) or Pointer.allocate is a recent change not yet part of a Dart release, but already in its master branch.

Copy link
Contributor Author

@Buggaboo Buggaboo Sep 5, 2019

Choose a reason for hiding this comment

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

$ dart --version
Dart VM version: 2.5.0-dev.2.1 (Thu Aug 15 16:05:39 2019 +0200) on "macos_x64"

I drink what homebrew offers me.

Copy link
Member

Choose a reason for hiding this comment

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

Uh, so FFI plans to change that!? Is there any way to support both version? If we cannot make this work for both, I think we should stick to the latest release version.

Copy link
Contributor Author

@Buggaboo Buggaboo Sep 5, 2019

Choose a reason for hiding this comment

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

I reverted the parts that are related to the fromAddress and allocate methods. That specific commit can be (re)reverted once 2.5 is the new stable.

@greenrobot greenrobot mentioned this pull request Sep 4, 2019
this triggered some weird compiler errors.
Then I tried this without explicitly casting, evidently
there's some type polymorphism going on, or some
casting by the compiler.

In any case, the Uint8List has been removed.
TODO Revert and test this commit  when 2.5 is released as stable
nalenz-objectbox added a commit that referenced this pull request Sep 6, 2019
@nalenz-objectbox
Copy link
Contributor

Great! Unfortunately the folder structure for the next version of this binding (0.2), currently in the dev branch, changed quite a lot, so I decided to merge your changes manually and close this PR. Also, because regarding FFI, Dart 2.4.1 and 2.5.0 are completely incompatible, we'll have a second branch called dev-2.5. Only it will contain a Flutter demo with Pointer.allocate instead of allocate etc.

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.

3 participants