Openvario / sensord

Daemon to poll air data from Openvario sensorboard
6 stars 11 forks source link

Added Voltage Offset #14

Closed hpmax closed 4 years ago

hpmax commented 4 years ago

This change modifies the current ads1110 code to add in an optional offset parameter read in the voltage_config line in sensord.conf. If the parameter is missing it defaults to a value of 0, and so is completely backward compatible with the current .conf file. The hardware has an offset and so it is impossible to calculate an accurate voltage without this offset value. This also immediately takes the reciprocal of the scaling factor read in, and then multiplies by the reciprocal instead of dividing -- presumably a minor speed up. This has been tested and works.

kedder commented 4 years ago

Btw, how do I pick those coefficients? Right now my sensord shows about 19V for my 12V power supply. How should I choose the right numbers for voltage factor and offset?

hpmax commented 4 years ago

Scale factor should be 1248.6 and offset should be 0.645.

If you want to experiment you can take an adjustable supply and try multiple points, and record the actual voltage (bear in mind if you are reading at the supply there may be a drop before it encounters the device), and the recorded output from the ADS1110 and then do a least squares fit.

hpmax commented 4 years ago

I have replaced "voltage_factor" with "scale" and "offset." I also changed one of the debug output lines so that it says voltage_scale = ... voltage_offset = ...." rather than "voltage_factor = ..."

kedder commented 4 years ago

Looks good to me!

hpmax commented 4 years ago

As a side note... I can't find any schematics for the adapterboard that include the A/D converter. The notes claim that they divide the battery voltage by 13 and then sample it. They claim that the older A/D converter (the ADS1100) uses a 3.3V reference, whereas the newer one (ADS1110) uses 2.048V. So the ratio between them should be 3.3 / 2.048.

I have no explanation for why there is an offset, but I can't help but note that 0.645 is roughly a diode drop. The schematic shows "Supply-1" going through a forward biased diode to "12V", so that might be the explanation. Of course, that means the 0.645V is temperature dependent. I produced a graph showing the reported voltage (with a voltage_factor = 736) on my ADS1110 vs actual voltage.

If it is a diode, and we should have the same offset value, so presumably its "774.888 0.645" I'd also love to know the reasoning. Even if you need a diode for some sort of reverse polarity protection, that voltage divider itself probably doesn't need it... you'd be connected to 13V through a 10k resistor, so your total current would be at most 150 microamps, I would think the body diodes and ESD diodes would keep things safe without messing up the voltage reading.

We should probably modify sensord.conf so that the default value is '1248.6 0.645" and put a comment next to it that if the battery voltage is low (7-9V range) / you are using the older ADS1100, change to "736" or more likely "774.888 0.645".

image

iglesiasmarianod commented 4 years ago

Hi guys. ADS configuration values are mentioned here: https://openvario.org/doku.php?id=projects:series_00:software:sensord 1200 for ADS1110 and 736 for ADS1100. Schematics can be found in the gerber files for adapteboard. https://openvario.org/doku.php?id=downloads (Schematics.tar.gz)

hpmax commented 4 years ago

I understand that these are the numbers that were published a long time ago, but...

It appears D1 is in fact in series with the power supply. It, along with D2 and the external fuse/breaker (and possibly the internal fuse) provides reverse polarity/overvoltage protection. I do not think it is necessary to put the R1/R2 voltage divider in series with D1, I think it would be safe to do it in parallel.

That said, the hardware is what it is. There is a diode in series and so the voltage will by definition have an offset. Like I said, a while ago, I swept the power supply and observed the output from the device with it set at 736 (I have an ADS1110), and produced the graph I showed above. I took the data and found that the reported value was 1.6965 * Actual voltage - 1.094... So if we do some algebra:

Reported = Actual 1.6965 - 1.094 (Reported + 1.094) / 1.6965 = Actual Reported = Measurement / 736 Measurement / (736 1.6965) + 1.094/1.6965 Measurement / 1248.6 + 0.64486 = Actual

And... those are the numbers.

The cost for the improved accuracy is literally one floating point add, and 4 bytes of memory.

linuxianer99 commented 4 years ago

@hpmax : The Codacy check showed up a new issue with this PR. The issues is not complete new as the scanff was there befrore, but it would be great if you can fix this please. Thanks a lot !! Afterwards i will merge, i promise :-) Thanks a lot !!

hpmax commented 4 years ago

I'm new to Github and Codacy. I saw it had complaints but I couldn't figure out what they were. Make griped about several things including ignoring return values and some legitimate bugs in sensorcal.c which I mentioned in the issues.

I've already finished integrating this fork into the temperature sensor one. So I'd really like to get this released and then roll all the additional fixes into the temperature sensor version.

hpmax commented 4 years ago

Fixed the Codacy issue (in all sscanf references).

Also, added the bugfix to sensorcal. Sensorcal has a 6 byte long string which is used for the serial number. The problem is that in C, strings are terminated by 0x00, so to have a 6 character long string, you'd need 7 bytes. As a result, the program overwrites calibration data. There are two solutions to this:

1) Increase string size to 7. This is a reasonable solution but it would move the location of some of the variables within the data structure, meaning you'd have to re-initialize the EEPROM. There is also an additional bug where it sets byte 8 of the string to /n -- this has to be removed.

2) The solution I opted for was to create a new variable called sn, which is a 7 character string. Initialize the last byte to 0, and then copy data.serial to sn -- memcpy(sn,data.serial,6) -- whenever data.serial is read.