search

tooyipjee / Spark-Analyzer

USB-PD ESP32 Power Analyzer
MIT License
118 stars 22 forks source link

WebApp_PPS does not remember configured values #7

Open doniks opened 3 months ago

doniks commented 3 months ago

repro:

As far as I can tell, WebApp does this just fine.

doniks commented 3 months ago

I think it's just due to the WebApp_PPS not using the Preferences class as WebApp does.

If that is all it is, then it should be simple to port this from WebApp to WebApp_PPS

doniks commented 3 months ago

Mhm, so I guess one extra challenge that I didn't see at first is how to store the PPS configuration versus the PD configuration.

I assume (haven't actually verified this) that the preferences are stored somewhere in a separate partition (can't find any partition table) and even when flashing PD vs PPS firmware (or if we would have one that supports both) then the same preferences partition will persist and that would mean the one firmware tries to write an integral value with the key "voltage" while the other firmware tries to write a float, this can't work, and even if it would, it's the whole point of PD vs PPS, that they do not support the same set of output values.

This leads me down a rabbit hole. I'd like to prefix the keys like this: ppsVoltage. In fact I would also prefer to change the variable names accordingly, and in fact I think the web api should be changed as well. On the one hand to make support of both PD and PPS in a single firmware in the future easier, but on the other hand to even avoid odd cases where the firmware has been reflashed, but the spiffs filesystem hasn't and it's serving incompatible values from the wrong web interface ...

So, all of that brings me to the thought that actually I want to change:

in both projects to keep it consistent. And in fact when I touch that I would like to think about a consistent naming scheme including the units V, A, mA .... aaaand actually I would like to also store the output state and actually the ... whatshallwecallit power supply mode "PS" vs "PPD"

any thoughts on this @tooyipjee ?

tooyipjee commented 3 months ago

Will try to incorporate some of these suggestions after the summer, thank you.

I the meantime too, happy if you/soemone wants to take point :)

doniks commented 3 months ago

I'm taking a stab at this. I'll try to post my draft working branch soon, will have to turn into a proper PR after.

But for now the rabbit hole seems to be getting a little deeper :-P

I've added more Serial.print to help me debug. However, I cannot drive PPS from my laptop directly. So, I have to plug the usb c into a power supply brick. Which means I cannot see the debug messages anymore. I was able to connect the second uart from the pin header, but the normal Serial.print() only goes to one .. (the first) initialized uart. So, now I'm looking at using ESP_LOGI, ESP_LOGD, etc instead of Serial because that one seems to go to both uarts simultaneously... tbc

doniks commented 3 months ago

very much work in progress branch: https://github.com/doniks/Spark-Analyzer/tree/wip just FYI

doniks commented 3 months ago

Ugh. No ESP_LOGD and friends don't really work the way I expect either. Apparently only ESP_LOGE is enabled and if I understand it correctly we can't run a esp menuconfig to change that ... I think something like arduino framework versus espressif...

doniks commented 3 months ago

ok, how about this one: https://github.com/tooyipjee/Spark-Analyzer/pull/10

want to test it a bit more before taking it out of draft status

doniks commented 3 months ago

updated PR: https://github.com/tooyipjee/Spark-Analyzer/pull/11

doniks commented 3 months ago

urgh. no GitGuardian didn't like my code, so here we go https://github.com/tooyipjee/Spark-Analyzer/pull/12