universam1 / iSpindel

electronic Hydrometer
http://www.ispindel.de
Other
826 stars 324 forks source link

Limit on polynominal setting field #452

Closed niki77 closed 3 years ago

niki77 commented 3 years ago

I'm submitting a ...

Did you follow the general troubleshooting steps first:

Report

In configuration i have discovered a limit of 100 char for polynominal setting.

Console Logs

Steps to Reproduce (for bugs)

  1. Calculated this polynominal value from ispindel tools htm page (calculated for SG) of degree 3: 0.8876047441253483 + 0.006888643779712255 *tilt-0.00011456539070270868 *tilt*tilt + 8.379435389370256e-7 *tilt*tilt*tilt
  2. Simplify that in : 0.8876047441253483+0.006888643779712255*tilt-0.0001145653907027086*tilt^2+8.379435389370256e-7*tilt^3
  3. Insert on configuration and save.
  4. When device restarted no SG value was displayed and checking in the configuration section the polynominal value entered before was truncated after the last 'tilt' and added by '000'
  5. I assumed there is a limit for this setting (100 char)and entered a more semplified polynominal value like : 0.8876047441253483+0.006888643779712255*tilt-0.0001145653907027086*tilt^2+8.3794353893702e-7*tilt^3
  6. When device restarted SG value was displayed correctly.

Context

Setting up my environment with calculated 3rd degree polynominal

Your Environment

Features

Please note by far the quickest way to get a new feature is to file a Pull Request.

ErikdBr commented 3 years ago

This is by design as per version 5.8.6. If a larger polynominal is wanted this should be a feature request, it's not a bug.

niki77 commented 3 years ago

If this is limitation by design i think the tool provided for the polynominal generation must not generate 'values' moreover the product capability. This in my opinion.

ErikdBr commented 3 years ago

The tool provided is https://github.com/universam1/iSpindel/blob/master/docs/Kalibrierung_en.xlsm Did you use this one?

thegreatgunbantoad commented 3 years ago

This one: https://github.com/universam1/iSpindel/blob/master/docs/ManualPolyGenerator.xlsx Produces a poly of 71 chars ish

It's lines 78 and 390 of iSpindel.cpp that fix it to 110 chars. I don't see any reason they couldn't be set to 1000. I'd think I'd rather see line 390 read: WiFiManagerParameter custom_polynom("POLYN", "Polynominal", htmlencode(my_polynominal).c_str(), sizeof(my_polynominal) * 2, WFM_NO_LABEL);

ErikdBr commented 3 years ago

Line 78 fixes it to 100 chars, not 110 that is why @niki77 's formula in the steps to reproduce point 2. is truncated because that is 102 chars long. I don't see any reason either why it couldn't be stretched to a bigger value, but someone has to do it :-) I don't control cpp in a way that I could do that nor do I unfortunately have the time to set up a development environment for it.

universam1 commented 3 years ago

Remember that double has a theoretical precision of 16 digits. Also especially for addition it is pointless.

You are welcome to submit an improvement of automation or documentation any time

thegreatgunbantoad commented 3 years ago

Line 78 fixes it to 100 chars, not 110 that is why @niki77 's formula in the steps to reproduce point 2. is truncated because that is 102 chars long. I don't see any reason either why it couldn't be stretched to a bigger value, but someone has to do it :-) I don't control cpp in a way that I could do that nor do I unfortunately have the time to set up a development environment for it.

Ahh yeah 110 was a typo. I have tried it at 1000 and everything works ok.

ErikdBr commented 3 years ago

So add it to the pull requests, but maybe 250 chars is more then sufficient? 1000 seems a bit overdone?

niki77 commented 3 years ago

For the calculation of the polynomial I used the online tool:

http://www.ispindel.de/tools/calibration/calibration.htm

o add it to the pull requests, but maybe 250 chars is more then sufficient? 1000 seems a bit overdone?

Yes i think it so, 250char is enough, but I don't know if the device resources are sufficient to expand the variable in question.

Currently I have solved the problem by reducing the precision of the third parameter. I have submit a change proposal.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.