ha-pitboss icon indicating copy to clipboard operation
ha-pitboss copied to clipboard

Possible to add target probe temperature?

Open DRuggeri opened this issue 1 year ago • 3 comments
trafficstars

Checklist

  • [X] I have filled out the template to the best of my ability.
  • [X] This only contains 1 feature request (if you have multiple feature requests, open one feature request for each feature request).
  • [X] This issue is not a duplicate feature request of previous feature requests.

Is your feature request related to a problem? Please describe.

The PitBoss app and physical controller allows setting and viewing the target temperature for the probe(s) attached to the grill. I am wondering if this data is broadcast in the BLE packets and if it could be set as a sensor value.

Describe the solution you'd like

It would be optimal to capture and report the target temperature(s) for the probes. I see that most of the information comes from a coordinator.data object, but I'm not savvy enough to understand how that object is populated via Bluetooth to detect what other keys are available in the data packet. A quick look shows that there are fields like data[f"p{self._probe_number}Temp"] and data["grillTemp"]

Describe alternatives you've considered

I examined the code to see if perhaps a debug log containing the data is logged at a lower level but it does not appear to be logged to know if this capability is possible. I'm happy to help debug/dump data/etc if recipes for doing so can be shared. I am using a ESPHome BLE Proxy for this integration, and can also stand up another proxy to get lower level information if that may be helpful?

For reference, I am using the pbv4ps2 smoker.

Additional context

Happy to help however I can! While I'm an experienced developer, my expertise with HASS integrations and Python magic is pretty weak - pointers to how to triage/debug would be appreciated!

DRuggeri avatar Aug 18 '24 17:08 DRuggeri

p1Target and p2Target are sometimes available in the state dict: https://github.com/dknowles2/pytboss/blob/main/pytboss/grills.py#L67-L71

Some grills support setting the target temps. I only implemented the ability to set probe1's target though: https://github.com/dknowles2/pytboss/blob/main/pytboss/api.py#L157 If you look through https://github.com/dknowles2/pytboss/blob/main/pytboss/grills.json you can see that some control boards support a set-probe-2-temperature, but most don't.

I think I didn't set all the probes as climate sensors because not all of them can have a target temp set. (Some grills have 4 probes attached but I think they only support target temps up to probe2) If you just want to read the target value, you can just add a new sensor type in https://github.com/dknowles2/ha-pitboss/blob/main/custom_components/pitboss/sensor.py. But we probably want to refactor that a bit since right now it's only set up for the probes themselves.

I'm not sure when I'll have time to look at this further, but if you want to take a stab at it, feel free to send a PR. :)

dknowles2 avatar Aug 30 '24 13:08 dknowles2

OK, yeah - that makes sense. You're correct in that I was only looking to read the probe target temperature. Your pointers help a ton!

I can see how I could create a targetTemperature sensor using sensor.py as a template. But... your point about refactoring makes sense because copying and pasting that whole file/class doesn't make a ton of sense given that we are primarily interested in just a few lines of changes (lines 44, 45, and 63).

It think the ProbeSensor class could be modified to accept a string in the constructor that would signal whether we want the current or target temperature from the dict, in the ID and also in the name. From there, in (what appears to be?) the HASS magic function to make it available, just register two types of sensors like so?

    for i in range(1, coordinator.grill_spec.meat_probes + 1):
        entities.append(ProbeSensor(coordinator, entry.unique_id, i, "Temp"))
        entities.append(ProbeSensor(coordinator, entry.unique_id, i, "Target"))
    async_add_devices(entities)

Does that track? Happy to make a PR for that.

Just a few additional thoughts:

  • Should I avoid changing attr_unique_id for Temp? As in... if that gets changed and the integration is updated, will it maybe break history graphs or something because the entity would get a different ID?
  • I have no idea how I can test it. I have a working pitboss grill and all that stuff, but what is the usual flow for validating this type of thing? Maybe just fork it and add a custom repository to HACS with my changes before PR?

DRuggeri avatar Sep 25 '24 23:09 DRuggeri

Yeah, that all sounds correct to me.

Should I avoid changing attr_unique_id for Temp? As in... if that gets changed and the integration is updated, will it maybe break history graphs or something because the entity would get a different ID?

Yeah, changing the unique id wouldn't be good. Maybe pass a unique_id_suffix parameter to the ProbeSensor constructor? For the "temp" sensors, it could just be an empty string for backwards compatibility.

I have no idea how I can test it. I have a working pitboss grill and all that stuff, but what is the usual flow for validating this type of thing? Maybe just fork it and add a custom repository to HACS with my changes before PR?

If you're developing in VSCode, it should prompt you to "Reopen in a Dev Container". Once it builds the docker image and puts you in the container, there should be a "Run Home Assistant on port 8123" option under the "Tasks: Run Task" command palette. (See: https://github.com/dknowles2/ha-pitboss/blob/main/.vscode/tasks.json)

(I should really write some unit tests for the integration, but I haven't had the time)

dknowles2 avatar Oct 09 '24 01:10 dknowles2