helgeerbe / OpenDTU-OnBattery

Software for ESP32 to talk to Hoymiles Inverters and Victrons MPPT battery chargers (Ve.Direct)
GNU General Public License v2.0
297 stars 62 forks source link

Allow higher-resolution SoC in live view header #1197

Closed ranma closed 3 weeks ago

ranma commented 1 month ago

Commit accc70dea0e4ead2f842f311d393530ad8fa0d55 added the battery SoC to the live view header. But due to getSoC returning an int the precision is currently limited.

This changes getSoC to return float so when a source with higher precision is available the full precision is shown.

AndreasBoehm commented 1 month ago

Good catch! @schlimmchen do you know anyone with a Victron SmartShunt, which is the only source which provides decimals for SoC, that could test this?

schlimmchen commented 1 month ago

I guess @ranma can show us a respective screenshot and then we'll believe that it works. However, I think we need to use %.1f as the format specifier when printing the SoC, afaik otherwise the float will print with way too many decimals?

ranma commented 1 month ago

I guess @ranma can show us a respective screenshot and then we'll believe that it works.

Not until I've set up an emulation test rig. I had to switch the battery back to victron protocol for the time being and I don't have remote access to the console.

However, I think we need to use %.1f as the format specifier when printing the SoC, afaik otherwise the float will print with way too many decimals?

Yeah, that's probably not a bad idea. For some files like PylontechCanReceiver.cpp it may even make more sense to store the integer temporary and keep the format as %d :)

ranma commented 4 weeks ago

Screenshot from replaying Victron BMV702 logs I found on https://github.com/karioja/vedirect/blob/master/test/bvm702.dump: image

$ stty raw 19200 < /dev/ttyUSB0 
$ cat /tmp/bvm702.dump  > /dev/ttyUSB0
ranma commented 4 weeks ago

However, I think we need to use %.1f as the format specifier when printing the SoC, afaik otherwise the float will print with way too many decimals?

Well, looking again the existing log printfs for voltage (which is already float with 2 decimals after the point) also uses just %f, so it should be fine as-is.