Example support for Burze.dzis.net v2.0.0 #203
Conversation
|
Approach used in this PR can also handle changed entity ids: |
There was a problem hiding this comment.
Hey! Thank you for this extensive PR! Detecting event type by unique_id is a great idea and the project will benefit from allowing integration methods to be asynchronous. I was not able to test it yet since there are no warnings today but, I've made a dry review for now.
| supports(hass: HomeAssistant, entity: HassEntity): Promise<boolean>, | ||
| alertActive(entity: HassEntity): boolean, | ||
| getAlerts(entity: HassEntity): MeteoalarmAlert[] | MeteoalarmAlert, | ||
| getAlerts(hass: HomeAssistant, entity: HassEntity): Promise<MeteoalarmAlert[]> | Promise<MeteoalarmAlert>, |
There was a problem hiding this comment.
I don't really like what you did here with passing hass object with each supports and getAlerts call. It just brings more clutter to these function calls. The better approach here might be passing hass object with the integration constructor and then storing it in the integration object
| @@ -27,9 +27,9 @@ export interface MeteoalarmCardConfig extends LovelaceCardConfig { | |||
|
|
|||
| export interface MeteoalarmIntegration { | |||
| metadata: MeteoalarmIntegrationMetadata, | |||
| supports(entity: HassEntity): boolean, | |||
| supports(hass: HomeAssistant, entity: HassEntity): Promise<boolean>, | |||
| alertActive(entity: HassEntity): boolean, | |||
There was a problem hiding this comment.
I like that you have moved all integration methods to be async but you can also do that alertActive It doesn't make sense that every method except this one is asynchronous
| export type EntityRegistryEntry = { | ||
| entity_id: string; | ||
| original_icon: string; | ||
| icon?: string; | ||
| unique_id: string; | ||
| disabled_by?: string; | ||
| } |
There was a problem hiding this comment.
You can map these snake_case varables into camelCase ones
| public static async getEntityInfo(hass: HomeAssistant, entity: HassEntity): Promise<EntityRegistryEntry> { | ||
| return await hass.callWS<EntityRegistryEntry>({ | ||
| type: 'config/entity_registry/get', | ||
| entity_id: entity.entity_id | ||
| }); | ||
| } |
There was a problem hiding this comment.
Please add jsdoc comment here this function looks extremely confiusing
| @@ -71,7 +78,7 @@ export class MeteoalarmCard extends LitElement { | |||
| ALLOWED_INTEGRATION_TYPES.includes(x.metadata.type) | |||
| ); | |||
| for(const integration of integrations) { | |||
| if(integration.supports(hass.states[entity])) { | |||
| if(await integration.supports(hass, hass.states[entity])) { | |||
There was a problem hiding this comment.
For code style, move all of these to be such as such:
const supports = await integration.supports(hass, hass.states[entity];
if(supports) {
...
}
|
This PR has been made mostly as a way of showing how the problem can be solved, not as an actual final solution - it was put together in a few hours as a proof of concept. Potential improvements and checks that need to be performed:
To test if the card displays data correctly you can set coordinates to |
So this is not the solution and you don't want to work on it? |
|
It can be treated as a starting point on a way to get a final solution. I can work on it, but unfortunately not at this moment, so you can take over development if you want to make it work sooner. |
|
Just to clarify; should I put it on my TODO list or do you want to take over development? |
Honesty, i dont know when I will get to it so i would say yeah, put it there. leet me know when you work on this i will do the same, will see who gets it done first |
|
@PiotrMachowski & @MrBartusek any hope to have burze.dzis.net working in HA AD2024? :) Either remove burze.dzis.net from MeteoalarmCard as a supported integration in readme.md, or move on with this PR :) it already hangs like 9 months, and windy times are coming to Poland :) |
|
@pskowronek |
yup, I think this was the post that led me here to make a ping in this PR :) |
Hi, I have made this short example how you can add support for Burze.dzis.net v2.0.0+ to your card.
Fixes #201