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

WIP - Property query #51

Closed
wants to merge 12 commits into from
Closed

Conversation

Buggaboo
Copy link
Contributor

#48 I need help with the tests, and some feedback wrt to the design.

My count test is broken. What am I doing wrong?

Please, don't merge yet.

@vaind
Copy link
Contributor

vaind commented Oct 23, 2019

My count test is broken. What am I doing wrong?

Couldn't find anything... current dart v2.5.2 still fails to properly show capture the exception (issue #1) and the lib doesn't compile with the latest dev v2.6 which should have the issue resolved and show you the proper error. I'll try to have a look at the errors given by 2.6 tomorrow and let you know.

@vaind
Copy link
Contributor

vaind commented Oct 23, 2019

Regarding the design, I'll have a better look tomorrow. Just one thing that occurred to me would be nice:

final queryInt = query.property(tLong); // lose the type in the method name
int sum = queryInt.sum();

Maybe we could handle it with the type system somehow - using generics on QueryProperty and getting the generic type out of it in the declaration of property() method

… e.g. "<T extends Base>")

* Replaced propertyX with generic property method (with auto-type inference)
* Added default to find functions, should the default arg value be 0
  or should the address be 0 for NULL? It would be nice to see the actual (closed) source code
  in this "open" source project.
@vaind
Copy link
Contributor

vaind commented Oct 24, 2019

I'll try to have a look at the errors given by 2.6 tomorrow and let you know.

Upgrading to 2.6 is not going to be that straightforward, quite a few things changed: #54
I've had another look at your code and it really seems OK, I don't see why it's throwing.

@Buggaboo
Copy link
Contributor Author

Regarding the design, I'll have a better look tomorrow. Just one thing that occurred to me would be nice:

final queryInt = query.property(tLong); // lose the type in the method name
int sum = queryInt.sum();

Maybe we could handle it with the type system somehow - using generics on QueryProperty and getting the generic type out of it in the declaration of property() method

Done.

@Buggaboo
Copy link
Contributor Author

My count test is broken. What am I doing wrong?

Couldn't find anything... current dart v2.5.2 still fails to properly show capture the exception (issue #1) and the lib doesn't compile with the latest dev v2.6 which should have the issue resolved and show you the proper error. I'll try to have a look at the errors given by 2.6 tomorrow and let you know.

It might be a bug wrt to signedness (e.g. UintX vs IntX), but I'm not sure.

@vaind
Copy link
Contributor

vaind commented Oct 24, 2019

With all the errors you're getting, it seems something is fundamentally wrong. I'd maybe wait for 2.6 before spending too much time trying to debug it. With a proper error message, it should be straightforward.

@Buggaboo
Copy link
Contributor Author

With all the errors you're getting, it seems something is fundamentally wrong. I'd maybe wait for 2.6 before spending too much time trying to debug it. With a proper error message, it should be straightforward.

Ugh. Can you please start a dev-2.6 branch? I'll start hacking that instead. This is driving me crazy.

@Buggaboo
Copy link
Contributor Author

Buggaboo commented Oct 29, 2019

I retraced my steps and reimplemented the cstructs used in the find calls without Struct, like we used to with OBX_array_ids. And I still have the same error. Something is fundamentally broken.

@vaind If you have time, could you take a look? Maybe there are issues wrt memory alignment, padding (anything smaller than the 64-bit word size) etc.

everything else (e.g. count) is still horribly broken
seems to return a result before the search is actually concluded,
there is some non-deterministic behavior here
@Buggaboo
Copy link
Contributor Author

Buggaboo commented Nov 6, 2019

Since 2.6 is the new stable version. I'm closing this one.

@Buggaboo Buggaboo closed this Nov 6, 2019
@Buggaboo Buggaboo deleted the query-property-dev branch November 6, 2019 10:13
greenrobot-team added a commit that referenced this pull request Mar 6, 2023
Attaching by ID is faster than attaching by path because time consuming
string operations can be avoided.
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