-
Notifications
You must be signed in to change notification settings - Fork 82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HomeAssistant Compatibility: Core Changes #533
base: development
Are you sure you want to change the base?
HomeAssistant Compatibility: Core Changes #533
Conversation
Signed-off-by: Andrew Andrianov <[email protected]>
Signed-off-by: Andrew Andrianov <[email protected]>
Signed-off-by: Andrew Andrianov <[email protected]>
Excellent thanks! Let me review the chances during the weekend and I'll let you know. Of course I will wait for a first full implementation before merging it into the development branch. |
Hi, I see a bunch of limitations to this approach which makes me think we should tackle the problem from a different angle, which is getting deep into the atomic data structure and change them radically if we want to adapt this The biggest one is Children are referenced by index in their underlying data structure so if you register a child with the same id, the previous one will be lost, regardless of checking for the type as well. Other more general guidelines, don't call child->sendValue() directly, but we need to find the way to go through sensor->loop() otherwise we lose PowerManager ad other stuff implemented there. I'm not a HA user but I also remember reading somewhere when you send a value for a sensor you need also to send the same empty just after, is that true? Thanks |
Hmm, I didn't notice that, guess I have to take another good dive in the code. Reworked dimmer code seemed to play nicely with those changes, so I assumed that it was working.
The current implementation just needs an initial value for an instance to show up. Nothing else. It will save it in a json file to persist across restarts, but since the file frequently overriten I keep it in tmpfs on my instance. I can post my quick and dirty nodemanager replacement I use now for my nodes if that can serve of any reference. On a side note. As far as I know, one of the core hass developers @MatinHjelmare is working on a new mysensors integration for HomeAssistant that will be based off a new library: https://github.com/MartinHjelmare/aiomysensors Perhaps we can just cooperate to make the new integration and library as painless as possible for all of us? |
Another idea here is an FSM, that will have a few states for each child:
As for the last state, my most recent experiments with a moderate (15 devices) network show that reliability becomes a problem for actuators once node is a hop or two away from the controller and guaranteed delivery is a must for actuators. Perhaps handling that would be best in the same FSM, but I'm still thinking about it. |
I see your point and appreciated it, yeah we need to think more strategically and potentially re-think the way NodeManager handles its sensors and children. I like the FSM idea. Sending out the initial value is not a big issue (actually this is done already when a reporting interval is set), the multiple types associated to the same ID is instead a bit more tricky. As soon as I will have some free cycles I'll pick my brain on it, definitely requires also for me a good deep dive into the code to remember how the different data structures are linked with each other. Thanks! |
The pull request implements the basic changes to NodeManager's code required for HomeAssistant compatibilty as discussed in #532
@user2684 You may want to have a detailed look at my changes. They seem trivial, but since I'm not very familiar with the codebase I might've left a few goblins lurking here and there.