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

VT send_change_size_command doesn't consider scaling factor #221

Open
GwnDaan opened this issue Mar 22, 2023 · 3 comments
Open

VT send_change_size_command doesn't consider scaling factor #221

GwnDaan opened this issue Mar 22, 2023 · 3 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request iso: virtual terminal Related to the ISO-11783:7 standard

Comments

@GwnDaan
Copy link
Member

GwnDaan commented Mar 22, 2023

By adding the dynamic object pool scaling in #164, we now have functions that don't take into account the scaling factor. Such as send_change_size_command:
https://github.com/ad3154/Isobus-plus-plus/blob/44a2941ca23f50f0a7ed07b4f74b460c91db0815/isobus/include/isobus/isobus/isobus_virtual_terminal_client.hpp#L575-L582

I can think of 2 possible solutions:

  • Make a public getter for the scaling factor, and let the user implement the scaling themselves
  • Automatically scale the dimensions inside the send_change_size_command

I'd prefer the latter, but both might be doable as well with a parameter option in the send_change_size_command function. E.g. automaticallyScaleDimensions.

@ad3154
Copy link
Member

ad3154 commented Mar 22, 2023

Ah, good thinking! I think I'd like to allow both if possible: setting an absolute size and auto-scaling. So maybe we should add a default parameter for scaling to some of these functions? Plus add a function to get the scale factor?

@GwnDaan
Copy link
Member Author

GwnDaan commented Mar 22, 2023

Ah, good thinking! I think I'd like to allow both if possible: setting an absolute size and auto-scaling. So maybe we should add a default parameter for scaling to some of these functions? Plus add a function to get the scale factor?

Okayy sounds good! I can work on this

@GwnDaan
Copy link
Member Author

GwnDaan commented Mar 23, 2023

We'd also need to take into account #223, as someone may also want to scale some object in a soft key using a command.

@GwnDaan GwnDaan added enhancement New feature or request iso: virtual terminal Related to the ISO-11783:7 standard bug Something isn't working labels Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request iso: virtual terminal Related to the ISO-11783:7 standard
Projects
None yet
Development

No branches or pull requests

2 participants