-
Notifications
You must be signed in to change notification settings - Fork 8
Unsubscribe callback handlers when updating a module #173
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
Comments
I agree. The callback system should definitely be reset completely when a new module gets loaded. Except for the event queue imo. For the persistant module. When that feature is implemented, I still wouldn't reboot the device on a module update because that is pretty slow. So I would still reset the callback system manually on module update. |
Actually, the decision of rebooting or not is dependent on the use case and should come from the developers. As of that, both options should be possible. |
@carllocos With the new issue system I'm going through the old issue backlog. For the reboot after module update, there are already a two debug operations that can help with this: halt and reset. imo this give developers enough control over this decision. I added the callback reset as a subissue (bug fix) and might have time to work on it soon. |
cool! I was not aware of For now, unsubscribing the callbacks (without reboot) is not a priority and if needed could easily be solved imo. Thanks for keeping it as a subissue. |
When updating a module, we should unsubscribe the callback handlers and this also means that for a MCU we should unsubscribe the service routine.
However, I believe that this may not even be necessary. The reason is that normally when we would update a module. The module should be persisted on the MCU so to survive potential reboots (I already opened an issue for this #121). If we persist a module and we decide to reboot the device, then the ISR get automatically unsubscribed. Meaning that this issue becomes irrelevant. However, I believe as long as we do not have the persisting feature yet, we should manually unsubscribe the ISRs.
What do you think @tolauwae?
The text was updated successfully, but these errors were encountered: