TJForc / risco-lan-bridge

risco-lan-bridge is a "bridge" between a node.js application and a Risco Panel
Other
21 stars 12 forks source link

Various bugfixes #6

Closed vanackej closed 2 years ago

vanackej commented 2 years ago

Here a few bugfixes, You can find the rational for each one in each commit.

vanackej commented 2 years ago

@TJForc I've also done an integration of your library with HomeAssistant using MQTT. Waiting for your approval on this to be able to publish it.

pergolafabio commented 2 years ago

Hi

where is that project ? is it the luca version ?

pergolafabio commented 2 years ago

@vanackej , do you mean this one? https://github.com/vanackej/risco-mqtt-home-assistant isnt that still using the riscocloud api?

is it also possible to create an version without mqtt ? like the one now availble?

vanackej commented 2 years ago

@pergolafabio The branch where I replaced riscocloud API with LAN API is here : https://github.com/vanackej/risco-mqtt-home-assistant/tree/feature/lan_communication

For the discussion regarding MQTT integration, have a look at the comment here : https://github.com/lucacalcaterra/risco-mqtt-bridge/issues/42#issuecomment-955199906

For the version without MQTT, well, it's this project ! I just made some adjustements in this pull request to fix some bugs and improve some behaviors, but no big changes here. I will maybe rewrite this project later using TypeScript and RxJS, because I'm more confortable with them and I think it could make maintenance and integration with third party libraries easier.

pergolafabio commented 2 years ago

Ah ok, sounds cool! So means we don't need an add-on? You gonna improve Onfreund his python version then?

vanackej commented 2 years ago

Don't need any addon on HA side, just using the standard https://www.home-assistant.io/integrations/alarm_control_panel.mqtt/ and https://www.home-assistant.io/integrations/binary_sensor.mqtt/ integrations to itnegrate the alarm panel and the sensors

pergolafabio commented 2 years ago

Perfect, but to run your project, it needs in a separate docker, right? Like Luca version?

vanackej commented 2 years ago

Yes, exactly. It will be the same, just based on the local API. I don't want to migrate it to Python, as I'm not a good Python developer and don't have time to do it. I also think an external service integrated through MQTT is more likely to get interest from various Home Automation communities : OpenHab, Jeedom, ...

Le sam. 30 oct. 2021 à 17:22, pergolafabio @.***> a écrit :

Perfect, but to run your project, it needs in a separate docker, right? Like Luca version?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/TJForc/risco-lan-bridge/pull/6#issuecomment-955317642, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAECPHLGYKR25R3S5GGHLSDUJQLTFANCNFSM5G74HUWQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

pergolafabio commented 2 years ago

Sounds good, can I contact you on discord?

robertboccia commented 2 years ago

We have been trying to get a risco solution in home assistant since the current integration broke when risco did an update a few months back so I hope this will solve the issues

Robert Boccia


From: pergolafabio @.> Sent: Saturday, October 30, 2021 6:05:08 PM To: TJForc/risco-lan-bridge @.> Cc: Subscribed @.***> Subject: Re: [TJForc/risco-lan-bridge] Various bugfixes (PR #6)

Sounds good, can I contact you on discord?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FTJForc%2Frisco-lan-bridge%2Fpull%2F6%23issuecomment-955391659&data=04%7C01%7C%7C7f9e9c0f504240ce413a08d99bbf0bcf%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637712067093629182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=45VlyreQbmdcRQ0u6v55%2FRWxS5IQITw9hYB%2FhLQOkZk%3D&reserved=0, or unsubscribehttps://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAR4IWBEGZTM4L2SKD32PBFDUJQQUJANCNFSM5G74HUWQ&data=04%7C01%7C%7C7f9e9c0f504240ce413a08d99bbf0bcf%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637712067093629182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=hg%2FoD6Il67oO8p1sv%2FJc3B%2FUQsLDJIXCY%2FDTTGwSctw%3D&reserved=0.

TJForc commented 2 years ago

@vanackej first of all, good job

I have 2 small remarks:

Otherwise in principle, your modifications seem correct to me and go in the right direction of the project.

I await your return on these points before merging.

TJForc commented 2 years ago

@vanackej Regarding the MQTT project, I see no objection, quite the contrary. This will avoid having to rewrite the code in python because like you, it is not a language that I master and I do not really have the time. Remember to update the README on your project, it still refers to the REST API.

You should also know that I am working on adding the possibility of using the RS-232 (or USB in the case of the Prosys) connection of the control panels supporting it for a direct physical connection without going through the network, which would allow users without a network module (or not wishing to use proxy mode with an old IP module while retaining the connection to RiscoCloud) to be able to communicate with their control panel. It will therefore be necessary to take into account this feature to come in your development.

When this is ready, the current project will be deprecated because I made the mistake of using the word 'lan' in its name while this feature will make the connection first and foremost 'local'. A dependency will also be necessary to provide management of a serial port. Because your project is based on a container, you will need to be able to map a serial port to this container.

Last point, as long as the communication with all the central units has not been validated, I will recommend that you do not assign a version 1.XX to your project but to stay on a version 0.XX because to this day I have not had feedback concluding that with LightSys and I absolutely cannot guarantee that communication will work with all types of power plants that theory suggests.

vanackej commented 2 years ago

@vanackej first of all, good job

I have 2 small remarks:

  • "Update internal state on Partitions before emitting events": you update the state of partitions and zones, I think we should also add for the outputs and the system to be more consistent.

Yes you are absolutely right. I will do the same for them too.

  • "Graceful disconnection ...": At this stage of the modifications, I do not see the need to add the export for the "RiscoPanel". I think that this last point is related to the automatic detection of the type of plant but that it is not yet required at this stage.

Yes, it was a mistake. It would indeed be required only with automatic panel type detection feature.

Otherwise in principle, your modifications seem correct to me and go in the right direction of the project.

I await your return on these points before merging.

OK. I should be able to have a look tomorrow.

vanackej commented 2 years ago

@TJForc I fixed all the points we discussed. I also corrected some javascript issues in order to cleanup the code base.

TJForc commented 2 years ago

I released version 0.10.4 which contains your changes. Take a look at the "package.json", it was well worth it.

pergolafabio commented 2 years ago

hi @vanackej , can you describe in the readme of your project how we can use it with home assistant? a guide is verry welcome, especially for HassOS users, we are limited... We can install a docker in portainer or would be verry usefull if you can port it to an addon so we can add it as an extra custom repository for now

thnx, appreciated!

robertboccia commented 2 years ago

I echo this, the reality is this is more than likely the biggest area that this new approach would have the biggest impact, there are many hundreds of users of the risco integration for home assistant who I think would benefit, the best way forward though I think would be to chat to @onfreund as he owns the current repo and has been looking for a way to take it local

From: pergolafabio @.> Sent: Tuesday, 02 November 2021 09:10 To: TJForc/risco-lan-bridge @.> Cc: Robert Boccia @.>; Comment @.> Subject: Re: [TJForc/risco-lan-bridge] Various bugfixes (PR #6)

hi @vanackejhttps://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fvanackej&data=04%7C01%7C%7C27135583da6748fc54b308d99dcfc12a%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637714337889536701%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=EuL%2FhtlFVXXkLgAqCWfEeFANB1ILHh5sBAhoSBrzLQ4%3D&reserved=0 , can you describe in the readme of your project how we can use it with home assistant? a guide is verry welcome, especially for HassOS users, we are limited... We can install a docker in portainer or would be verry usefull if you can port it to an addon so we can add it as an extra custom repository for now

thnx, appreciated!

- You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FTJForc%2Frisco-lan-bridge%2Fpull%2F6%23issuecomment-957153924&data=04%7C01%7C%7C27135583da6748fc54b308d99dcfc12a%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637714337889546702%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=3ShzF39pZRw57vSbX4GpKNUn33jGe%2BrJ0BXIvHI%2Bbh4%3D&reserved=0, or unsubscribehttps://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAR4IWBGTCBQO3ANRDXPN3PTUJ6FDVANCNFSM5G74HUWQ&data=04%7C01%7C%7C27135583da6748fc54b308d99dcfc12a%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637714337889556690%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=7j7rw4I0dvEQhNvExvagfaxgTED7L%2BAU6qAK9VdmB8A%3D&reserved=0.

pergolafabio commented 2 years ago

yeah, but onfreund edition is a python version... its not converted to python yet, so the best approach for now is an add-on ... untill it gets converted to python ... am i right? :-)

vanackej commented 2 years ago

@TJForc Thanks for the mention in the contributors, very much appreciated 👍

@pergolafabio Regarding the HA integration, I think it's a little too early to make it public and documented. I want first to have the panel type autodetection feature, as it will allow for simpler configuration and packaging.

I also have some issues with my local MQTT broker that does not honor the qos and retain flags => I'm forced to republish the online/offline status more frequently as I wish.

I want to fix all those points before making the project public, as I can't spend much time making support.

pergolafabio commented 2 years ago

Ah ok, no problem... For now we can still use Onfreund edition, it works also

OnFreund commented 2 years ago

Thanks @robertboccia for tagging me.

I'm very much looking forward to having local support in PyRisco. Unfortunately, I don't have the capacity to take it from 0 to a 100 at the moment, but I'm happy to both review and contribute PRs as part of a larger effort, if we have other people also supporting this. If we maintain the same interface in PyRisco (where you can either init a local connection or a cloud connection, but get back an object that behaves the same and abstracts the connection method), there will be minimal changes to the HA integration - mostly just allowing the user to configure the type of connection and relevant parameters. The rest stays mostly the same.