Skip to content

Conversation

@lawrencegsullivan
Copy link
Contributor

@lawrencegsullivan lawrencegsullivan commented Feb 14, 2024

What

Don't demote doubles to floats in varGetValue

Why

Greater precision is better, maybe?
Sometimes we want to display double precision numbers on our HMIs

If y'all need double precision this might be the right starting place, however there may be additional changes required that I haven't considered yet.

I have not tested in the LoupeUX stack.

@lawrencegsullivan lawrencegsullivan added the enhancement New feature or request label Feb 14, 2024
@lawrencegsullivan lawrencegsullivan changed the title Feature/improve lreal support Improve LREAL support Feb 14, 2024
@lawrencegsullivan
Copy link
Contributor Author

Within the LoupeUX stack there are comms bandwidth considerations here as well.

@lawrencegsullivan lawrencegsullivan changed the title Improve LREAL support Improve LREAL support (WIP) Feb 14, 2024
@shanereetz
Copy link

Conceptually this change seems like an improvement, in that the previous behavior seems to prevent users from getting LREALs into traces, or other parts of the HMI.

The bandwidth considerations are also important, and how this might impact existing applications if they were to update, especially if they are using many LREALs that are currently being demoted. Could we test performance with an existing application that uses many LREALs for comparison?

Lastly, I noticed that this PR has the TLSF updates as well, was that intentional or meant to be in a separate PR?

@shanereetz
Copy link

Actual last thought: would it be worth offering some kind of flag to opt-in to LREAL support?

@lawrencegsullivan
Copy link
Contributor Author

Lastly, I noticed that this PR has the TLSF updates as well, was that intentional or meant to be in a separate PR?

StringExt v0.15.0 uses TLSF and builds with it as a static library. StringExt binaries were brought into the example project including a tlsf header file. This shouldn't impact VarTools.

@shanereetz
Copy link

LGTM but given my minimal experience with this repo, I'll pass approval to someone with more internal knowledge. Thanks for doing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants