fix: update mqtt subscriptions when topic was changed

[AndreasBoehm]

Update mqtt subscription and update homeassistant config when topic was changed.

Fixes #1045

[schlimmchen]

Looks good, but it's a little too much to review on the phone. Is a review necessary? I tend to trust that this is good.

Are all OnBattery extensions covered with this?

[AndreasBoehm]

I went through all classes but because of all the back and forth i would not trust myself that i covered all OnBattery extensions with this.

Let's wait and get it fully reviewed.

[schlimmchen]

As far as I can tell, you covered all relevant subscriptions. However, I am not happy about duplicating each subscription string to implement the unsubscribing for two reasons: (1) occupies flash storage and (2) when adding new topics the module subscribes to, there is a high chance that the unsubscribe function is not taken care of as well.

What do you think about this idea: In the respective lambdas that call MqttSetting.subscribe() we know the full topic to subscribe to. After subscribing, we add the topic string to a container (I guess a vector). The unsubscribe() function iterates all these topics and calls unsubscribe for them. I know this uses more RAM, which is the only thing that makes me unsure that this is the right approach.

How about a frozen::map? That would live in flash, would be compact (no strings are defined two times), and we could iterate it to subscribe and to unsubscribe?

I just added a respective change. What do you think?

[AndreasBoehm]

I really like the idea, makes sense to do it that way 👍

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion or issue for related concerns.