Expose additional sensors, add more accessory modules for H2 series#1953
Expose additional sensors, add more accessory modules for H2 series#1953rhooper wants to merge 5 commits into
Conversation
|
To view this pull requests documentation preview, visit the following URL: docs.page/greghesp/ha-bambulab~1953 Documentation is deployed and generated using docs.page. |
| ("_bot_err", "bot_err"), | ||
| ("_cd", "cd"), | ||
| ("_cd_init", "cd_init"), | ||
| ("_drill_flag", "drill_flag"), |
There was a problem hiding this comment.
Are all of these valuable to expose? It looks like you aren't using all of them?
There was a problem hiding this comment.
In my updated (and rebased) push, they're now exposed so I can monitor for their values.
| @@ -3575,9 +3707,26 @@ def print_update (self, data): | |||
| self.state = "laser40" | |||
| elif mount == 1 and tool_type == "CP00": | |||
| self.state = "cutter" | |||
| elif mount == 1 and tool_type == "F000": | |||
| self.state = "cooling_fan" | |||
| elif mount == 1 and tool_type: | |||
| self.state = None | |||
|
|
|||
| self.state = "unknown" | |||
|
|
|||
| # Parse additional fields | |||
| if "th_temp" in ext_tool: | |||
| self.th_temp = ext_tool["th_temp"] | |||
| if "calib" in ext_tool: | |||
There was a problem hiding this comment.
This can be made more compact to handle the delta mqtt payloads as:
self.calib = ext.tool.get("calib", self.calib)
There was a problem hiding this comment.
I rewrote it to use this style, then claude pointed out that I'd be clobbering values on partial mqtt updates causing oscillations between None and being set.
There was a problem hiding this comment.
self.calib = ext.tool.get("calib", self.calib)
This means get("calib") and if not present, set self.calib = self.calib (no change)
So I don't know what Claude was complaining about.
| device_class=SensorDeviceClass.ENUM, | ||
| options=["standby", "active"], | ||
| value_fn=lambda self: self.coordinator.get_model().fire_extinguisher.status, | ||
| available_fn=lambda self: self.coordinator.get_model().fire_extinguisher.connected, |
There was a problem hiding this comment.
In general I'm trying to move away from making entities unavailable (due to people that complain about all unavailable entities). If not connected, is that a permanent state or can it change live? If it's more permanent - this should be driving the exists check. But none of these sensors have an exists_fn even though you are trying to call that - which means it's always going to be returning true.
There was a problem hiding this comment.
The fire extinguisher, rotary tool, external pump, cutting tool, and laser tool are all hot connectable. Is the correct fix to implement exists_fn?
There was a problem hiding this comment.
No, if they're hot connectable then unavailable is probably the best option. But you do need to implement exists_fn because most of the bambu models do not have this and never will. exists needs to return false for those models.
There was a problem hiding this comment.
You have supports_features check at device creation - move that into the exists check instead.
There was a problem hiding this comment.
On this topic -- are the devices only created when they are first seen as connected? A laser or fire extinguisher device doesn't make sense for H2 owners that don't have the laser edition/upgrade.
If you'd like me to test (I have an H2D and bought the cutter on sale last year), let me know when you get an update made.
There was a problem hiding this comment.
Yes, we can probably do better than that - I re-initialize sensors in certain scenarios already - such as when the user toggles the camera on/off so that the entity goes away when it's off. We'll need an event that the pybambu layer raises that would trigger the coordinator to refresh the entities. Then that event can be raised each time a hardware change occurs like this.
There was a problem hiding this comment.
Would be more work to extend that to an entire device though - I have logic to detect and delete stale AMS's. It should be possible in principle though. Not sure whether entire devices coming/going would be the best experience though.
There was a problem hiding this comment.
This is getting off topic from the review. Maybe we should take the discussion elsewhere.
For background, I've been using Home Assistant for years; my trade is software engineering and cybersecurity. My instance is on the larger side -- about 160 automations, 80 user-facing integrations (21 custom), 8000+ entitles, 4600+ enabled. At this scale, the more consistently integrations behave the better; the Integration Quality Scale is the HA developers' vision of what a good integration is and what the built-in integrations are using to advance the experience. My comments are rooted in that scale.
"Unavailable" is well-specified in the quality scale and likely other documentation/community posts, it is a legitimate state and means that the entity is known but data cannot be fetched. (Which lines up with a tool's entities when that tool has been seen but is not connected.) This state has a few rough edges, but there is enough in HA to manage it. Updating my templates and automations to consider 'unavailable' has made my system more reliable (in terms of not doing weird things.)
The ideal time to create devices is when they are connected. This is a Gold-level rule and commissioning new devices is relatively uncommon; if a new device doesn't show up immediately I'm OK with restarting Home Assistant or the integration to make it work.
The right time to remove devices/entities is also specified: when the integration knows the device is no longer present. When there's a hub or account this is clear as the hub/account can be interrogated. It's less clear for a Bambu Lab printer when there's only a LAN connection. As this is another Gold-level rule and decommissioning is uncommon, it's OK if I have to manually remove devices that I've stopped using. I'd rather be intentional about that than have entities I depend on disappear.
Taking a couple of cases from ha-bambulab:
- An entity that's always connected to the printer, like the camera, is unlikely to be decommissioned. The user might have turned off LiveView at the printer, but after a power cycle, it's much more likely the camera has not yet initialized (or failed). Home Assistant keeps the camera in the entity registry, so delayed creation is not fatal, but in that case the entity is not included in (integration_entities())[https://www.home-assistant.io/template-functions/integration_entities/].
- For pluggable accessories that I own like AMSes,, cutter, etc. I'd expect the devices to remain in Home Assistant indefinitely. This avoids problems with running scripts/automations, and enables working on automations without the device connected. Even if the accessory is not seen for months or many runtime hours, it could be that I only use that tool a few times a year. Offering a repair might be appropriate though, ss it leaves the decision to the user.
(P.S. Even further off topic, I have a repair automation that reloads bambu_lab if the camera remains unavailable, so I don't know if the camera delay is the integration being dynamic or just my reload. Either way it's working well enough for me, now.)
There was a problem hiding this comment.
The AMS stale detection is because it's legal and not that uncommon for people to reconfigure AMSs between their printers. If I don't remove them from the printers they are no longer connected to, things end up being very broken for that user. IIRC I only remove an AMS when it's been seen connected to the current instance and I can see orphaned connections to other instances of the integration (but I'll check - I may be misremembering). Which I think should handle both scenarios - an AMS going offline for a while shouldn't disappear but moving an AMS should move.
Are the tools uniquely identified such that they can also move between printers?
There was a problem hiding this comment.
Deferring creation of the devices until they are connected is definitely the best choice as I suspect the vast majority of owners will never get them.
…evices Parse device.fire_ext MQTT data and get_version module info (afp, lc, lfa) to create proper child devices with serial numbers, hw/sw versions. Tool modules are tracked by serial so multiple lasers/cutters create distinct devices. Entities show unavailable when their module is disconnected, and devices persist until manually removed by the user. New devices and entities: - Fire Extinguisher: status, remaining time, error sensors - Laser Module: temperature sensor, mounted and low precision binary sensors - Rotary Attachment: mounted binary sensor Also enhances ExtruderTool to parse th_temp, calib, low_prec, mount_3d fields and adds "unknown" fallback for unrecognized tool types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mount_3d is a mode flag, not physical connection state. The actual rotary tool connection is reported via device.fourth_axis.connect_flag. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| if sensor.exists_fn(coordinator): | ||
| async_add_entities([BambuLabFireExtinguisherSensor(coordinator, sensor)]) | ||
|
|
||
| for module in coordinator.get_model().extruder_tool.get_modules_by_type("laser"): |
There was a problem hiding this comment.
Needs a supports_feature check to limit this to models that have this
There was a problem hiding this comment.
Or preferably implement the exists_fn method and add the feature check there.
| device_class=SensorDeviceClass.ENUM, | ||
| options=["standby", "active"], | ||
| value_fn=lambda self: self.coordinator.get_model().fire_extinguisher.status, | ||
| available_fn=lambda self: self.coordinator.get_model().fire_extinguisher.connected, |
There was a problem hiding this comment.
You have supports_features check at device creation - move that into the exists check instead.
At least the entities doc would benefit from an update; maybe more as the list of devices is growing. |
3 participants
Description
Exposes additional sensor values, splits out the accessories for the H2 series into their own devices. Looking for feedback before finalizing.
Type of Change
Adds
Link to Issue
n/a
Documentation
Testing
Additional Notes
Claude was used here.