Skip to content
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

Battery gauge fix #52

Merged
merged 4 commits into from
Sep 12, 2020
Merged

Battery gauge fix #52

merged 4 commits into from
Sep 12, 2020

Conversation

rochuck
Copy link

@rochuck rochuck commented Sep 10, 2020

This PR fixes the battery voltage reporting to the Web UI. status.json was reporting an integer value for the battery voltage. The ranges have also been updated to what I think are more reasonable values.

@sle118
Copy link
Owner

sle118 commented Sep 11, 2020

@philippe44, I assigned the review to you since you have a better point of view from the SqueezeAmp angle!

…ge-fix

* upstream/master-cmake:
  WIP User Interface improvement. Fix SqueezeAmp build - release
  WIP - User Interface improvements
@philippe44
Copy link
Collaborator

I'm fine with the C change, the JS part is really your realm @sle118

@sle118
Copy link
Owner

sle118 commented Sep 12, 2020

There's a formatting hell here, making it difficult to figure out the difference. I'll take some time to review other the weekend.

@rochuck thank you for submitting this

@rochuck
Copy link
Author

rochuck commented Sep 12, 2020

@sle118 I did do a merge, so the diff should be straight forward, unless I'm missing something. If you are in the middle of JS changes, I can wait till you're done and re-merge. @daduke may wish to weighing in on this, since I think he coded the original voltage ranges.

@sle118
Copy link
Owner

sle118 commented Sep 12, 2020

Would you mind pulling the latest changes and reapplying you changes, please? To save flash space, I completely reformatted the js file with tabs instead of spaces, and that makes it difficult to spot the changes.

@rochuck
Copy link
Author

rochuck commented Sep 12, 2020

@sle118 This should be fine as is. How are you looking at the changes? The diff here: https://github.com/sle118/squeezelite-esp32/pull/52/files is pretty clean, to my eyes.

@sle118
Copy link
Owner

sle118 commented Sep 12, 2020

Ok, I see it now. I was looking at the wrong place

@sle118 sle118 merged commit cc5fb49 into sle118:master-cmake Sep 12, 2020
@sle118
Copy link
Owner

sle118 commented Sep 12, 2020

Additional comment: the status.json handles numerical values with a double internally, so there shouldn't be a change needed in the status cJSON calls.

rochuck added a commit to rochuck/squeezelite-esp32 that referenced this pull request Sep 14, 2020
…to master-cmake

* 'master-cmake' of github.com:sle118/squeezelite-esp32:
  fix a couple of gpio names (SPI/I2C)
  New config UI for Services (Airplay, bt, etc) - release
  Add nvs "wifi_ps" to disable wifi power save mode - release
  Battery gauge fix (sle118#52)
rochuck added a commit to rochuck/squeezelite-esp32 that referenced this pull request Sep 14, 2020
* master-cmake:
  fix a couple of gpio names (SPI/I2C)
  New config UI for Services (Airplay, bt, etc) - release
  Add nvs "wifi_ps" to disable wifi power save mode - release
  Battery gauge fix (sle118#52)
  WIP User Interface improvement. Fix SqueezeAmp build - release
  WIP - User Interface improvements
@rochuck rochuck deleted the battery-gauge-fix branch November 3, 2020 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants