-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
VOCKQJK11LM Improvements and modernExtend conversion (2/4) #6739
Conversation
cba43b3
to
eab73c7
Compare
2b6a583
to
3e1eba3
Compare
@Koenkk looks like somehow with the current code the options for temperature calibration are already somehow exposed. I don't understand where it's getting it from though. Do we just automatically add them for On the upside, seems the calibration is working fine with the changes so far! I |
@sjorge I don't understand it either, but anyway the whole calibration should be moved up (outside of the converters) |
Where do you want the calibration to end up ? |
3e1eba3
to
6f51890
Compare
There should be some "post process" function (out of scope for this PR), thinking about it now, you can remove the whole calibrate from your code, since the calibrate options will be discovered automatically based on the exposes. |
Having a post process chain would be very nice, it could also fix things like So I just drop the calibrate stuff and the PR is good to go I guess ? |
6f51890
to
63af9fe
Compare
Cleaned up the calibrate stuff. |
Thanks! |
Still very WIP but I though I'd open as draft already.
I plan on extending the modernExtend numeric with a 'calibration' argument that will toggle the calibration options.
It would be nice to have that code available for all numerics so we can easily enable where it makes sense.