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

Fix parsing of double value when QuantityType is used #56

Merged
merged 4 commits into from
Oct 14, 2022

Conversation

seime
Copy link
Collaborator

@seime seime commented Oct 9, 2022

... as annotation comparator parameter

@seaside1
Copy link
Owner

seaside1 commented Oct 9, 2022

But this will be for more than just no QuantityTypes? I have not tested this myself but will QuantityType print a similar error message?

@seime
Copy link
Collaborator Author

seime commented Oct 10, 2022

The catch clause catches everything, but could be reduced to only NumberFormatException.

In general I've been thinking about consolidating all the mapping methods between JRule<->openHAB scattered around into 2 classes, one for each way. Easier to reuse then.

@querdenker2k
Copy link
Collaborator

Why are the openHAB classes are all mapped at all?
Why not use the openHAB classes directory especially the types/states?

@seaside1
Copy link
Owner

@querdenker2k Initially I started with that, to have full control and minimize dependencies. Also it's a bit more clear what is beloging to jrule and what is core. I realize that using the coretypes could would have worked, it is just a choice to try out with the mapped classes.

This is something we could change, if we want to. I feel the strong typing is a good idea, it's really simple to develop rules I have to say.

@seaside1 seaside1 merged commit eb139ff into seaside1:main Oct 14, 2022
@seime
Copy link
Collaborator Author

seime commented Oct 21, 2022

I'd also prefer to use the core classes (states and commands) directly and avoid mappings if possible.

@seaside1
Copy link
Owner

I'm not against it. Main drawback for me is that it requires more deps as a rule developer but that's maybe not a big one.
Also the core classes tend to be a bit diverging from each other. But yes will def remove a lot code where mapping between types are done.

So it's quite a big refactoring to do, I guess it could be done incrementally if we want to do it.

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.

None yet

3 participants