Main API cleanup. Breaking changes#198
Conversation
* class Zaptec * Make class Zaptec an async context in order to close the web session * Make the class a `Mapping` of (Zaptec) objects with dict-like interface. * Mapping changes must go via `register` and `unregister`. * `register` will fail on re-registeringing an existing object * Add `.objects()`, `.installations` and `chargers` as methods to fetch subset of data from the class * `.build()` can be called multiple times. If run again it will update the existing objects * Stop exposing `.map` * Remove `get_circuits_id()`. Its too specialized and is handled directly where its used. * `.get_chargers()` is not the property `.chargers` * `.installations` is now a property and not a variable * Add warnings on removal of or standalone chargers. We don't handle this yet. * class ZaptecBase * Make class ZaptecBase a `Mapping` with a RO dict-like interface for parameters * Removed `.__getattr__()` so `obj.something` doesn't work. However `.id` and `.name` is explicitly kept. * class Installation * Update `.build()` to support re-runs without creating new objects * class Charger * Remove unused `.live()` method * Remove empty `.build()` method * General * Cleanup and reorganization * Adopt the HA integration to the API changes
|
@steinmn I'm shooting from the hip here, but I would really appreciate if you could take a look at this one. It's not a small one, so I'm grateful if you have the time and opportunity. This is out of the blue, so please feel free to ignore if it doesn't. It's a major lift on the overall methods and API for the classes in the Zaptec-facing api. |
steinmn
left a comment
steinmn
left a comment
There was a problem hiding this comment.
I'm a bit too unfamiliar with async to fully grasp everything going on, but apart from a couple of nitpicks, the rest looks good to me 👍
custom_components/zaptec/api.py
Outdated
| _LOGGER.warning("To remove them, please restart the integration.") | ||
| if extra_chargers := (new_chargers - have_chargers): | ||
| _LOGGER.warning( | ||
| "These standalone chargers will not added: %s", extra_chargers |
There was a problem hiding this comment.
| "These standalone chargers will not added: %s", extra_chargers | |
| "These standalone chargers will not be added: %s", extra_chargers |
There was a problem hiding this comment.
Fixed. Good catch.
custom_components/zaptec/api.py
Outdated
| charger = Charger(charger_item, self.zaptec, installation=self) | ||
| self.zaptec.register(charger_item["Id"], charger) | ||
|
|
||
| await charger.build() |
There was a problem hiding this comment.
Do we need this? charger.build() still does nothing, right?
There was a problem hiding this comment.
I first argued that it should be there for consistency. But then I was thinking that if we need something to happen in Charger.build() we have to run a PR for it, so why not also update this part then? So I removed it.
2 participants
class Zaptec
Mappingof (Zaptec) objects with dict-like interface.registerandunregister.registerwill fail on re-registeringing an existing object.objects(),.installationsandchargersas methods to fetch subset of data from the class.build()can be called multiple times. If run again it will update the existing objects.mapget_circuits_id(). Its too specialized and is handled directly where its used..get_chargers()is not the property.chargers.installationsis now a property and not a variableclass ZaptecBase
Mappingwith a RO dict-like interface for parameters.__getattr__()soobj.somethingdoesn't work. However.idand.nameis explicitly kept.class Installation
.build()to support re-runs without creating new objectsclass Charger
.live()method.build()methodGeneral