Add thermo sensor selection to Tap Settings #120
Conversation
There was a problem hiding this comment.
I'll give this a good test tonight.
What's up with all the whitespace changes? Any chance those can be broken out from this feature PR?
Do we have a formatting / linting standard for java like we have with black on https://github.com/Kegbot/kegbot-server?
|
Yeah @patfreeman that's the auto-indent that Android Studio applied, but I agree, I hate it as far as consistency and readability of the code changes. Please give this a functional test if you can, and in the meantime I'm going to re-factor this to get rid of all that whitespace stuff. |
|
cool feature + awesome collaboration here! on the lint stuff - if we need to do a great big "run the linter on everything" sort of change, i'd be happy to merge - best to do that with no other functional changes in the pull request. |
|
@johnnyruz - to get rid of the whitespace changes, something like the following could help do the trick. starting point would be something like: then (the no whitespace view of the current change looks quite sensible & could be compared against) |
|
I was trying to slack him another solution, but then this happened: https://status.slack.com/2020-05/147dad376c8946ff |
| @@ -49,6 +49,7 @@ | |||
| import org.kegbot.app.util.Version; | |||
| import org.kegbot.backend.Backend; | |||
| import org.kegbot.backend.BackendException; | |||
| import org.kegbot.core.ThermoSensor; | |||
There was a problem hiding this comment.
This is an unused import.
You should import org.kegbot.proto.Models.ThermoSensor instead. Then you can change the references from Models.ThermoSensor to just ThermoSensor.
| @@ -707,6 +708,11 @@ public KegTap disconnectMeter(KegTap tap) throws BackendException { | |||
| return (KegTap) postProto("/taps/" + tap.getId() + "/disconnect-meter", KegTap.newBuilder(), null); | |||
| } | |||
|
|
|||
| @Override | |||
| public List<Models.ThermoSensor> getThermoSensors() throws BackendException { | |||
| return getProto("/thermo-sensors/", Models.ThermoSensor.newBuilder()); | |||
There was a problem hiding this comment.
See above comment about using ThermoSensor instead.
| } | ||
|
|
||
| @Override | ||
| public KegTap connectThermo(KegTap tap, Models.ThermoSensor thermo) throws BackendException { |
There was a problem hiding this comment.
See above comment about using ThermoSensor instead.
| @@ -52,6 +52,7 @@ | |||
| import org.kegbot.core.KegbotCore; | |||
| import org.kegbot.core.SyncManager; | |||
| import org.kegbot.core.TapManager; | |||
| import org.kegbot.core.ThermoSensor; | |||
There was a problem hiding this comment.
Same situation as KegbotApiImpl.java.
| */ | ||
| package org.kegbot.app.event; | ||
|
|
||
| import org.kegbot.proto.Models; |
There was a problem hiding this comment.
This import could be more specific since you are only using Models.ThermoSensor
| @@ -24,6 +24,7 @@ | |||
| import org.codehaus.jackson.JsonNode; | |||
| import org.kegbot.app.util.TimeSeries; | |||
| import org.kegbot.proto.Api.RecordTemperatureRequest; | |||
| import org.kegbot.proto.Models; | |||
There was a problem hiding this comment.
This import could be more specific since you are only using Models.ThermoSensor
|
Love the feedback, working on it now! The suggestion about the whitespace sort of worked, but I dug into the formatting settings of Android Studio and updated some of the policies so that I can just apply that setting and the indentations line up. Latest will only have real code changes and all of the whitespace mess will be cleaned up. |
|
Updated code with whitespaces fixes and suggestions from @patfreeman |
|
thanks johnny, and thanks for handling the review pat! LGTM |
Combined with PR #409 on kegbot-server, this will allow users to manage temperature sensors from the Android app similar to Flow Meters and Tap Toggles.
Sorry for all of the irrelevant tabbing, looks like Android Studio adjusted some of the indentation.