Provide a setting to define location for non-HA topics (#102)

[thomasddn]

I provided a setting mqtt_options.volvo_topic with a default value of homeassistant/[domain], where [domain] will be replaced with the HA domain. This default value comes down to the same value as before. The [domain] part is optional.

If I run the container with these changes, I see all topics being created according to the new setting.

If you have any suggestions, I'm happy to apply them.

[Dielee]

First, thanks for your PR. Looks good! Please change the few things. I will merge into a test branch and test a few things after this.

[thomasddn]

First, thanks for your PR. Looks good! Please change the few things. I will merge into a test branch and test a few things after this.

Where can I find the changes you want? I don't see anything in the review conversations. Am I missing something?

[Dielee]

I started a file review. Just check the changed files in this PR.

[thomasddn]

The thing is that I don't see any of your comments. Nothing under "Conversations" nor anything under the files. Just Googled a bit. Did you "submit or finish" your review? See second alinea of the first comment on https://github.com/orgs/community/discussions/30638.

[Dielee]

My bad, wait a sec

[Dielee]

Please also update the readme file!

[thomasddn]

I also commented on the config.yaml file in the review window. I find the review UX a bit confusing, so that's why I mention it here as well, in case you missed it.

[Dielee]

Any news here?

[thomasddn]

Any news here?

Yes, I just thought you were busy. I have processed your comments and committed them. But we were still discussing the changes for config.yaml: https://github.com/Dielee/volvo2mqtt/pull/109/files/53ad010a9c2c298e1d79e4197b8e227bd9337fdc#r1337219343

[Dielee]

Those are the open topics:

https://github.com/Dielee/volvo2mqtt/pull/109#discussion_r1337182612

https://github.com/Dielee/volvo2mqtt/pull/109#discussion_r1343981083

If we fix this, I will merge into dev.

[thomasddn]

@Dielee Unfortunately those links lead to nowhere. I can just see a preview when I hover over the link, but not enough to make something of your comment. I also checked the conversation view under the "files changed" tab, but can't see anything new.

Very weird.

See screenshot below. But when I click on the link, the page doesn't change.

[Dielee]

Really weird... If your change for the state topics also works for HA and Autodiscovery, we should remove the option base_topic to change the state topic. Just simply volvoAAOS2mqtt. If this works for HA and every none HA user, this should be the best option.

[thomasddn]

Just making sure I understand: we should not make it an option and just move everything under volvoAAOS2mqtt, except for what is needed by HA. Is that correct?

That is perfectly possible, but it does mean a breaking change. Like mentioned here: https://github.com/Dielee/volvo2mqtt/issues/102#issuecomment-1722459486

If you are OK with that, then I will change the implementation accordingly.

[Dielee]

Right. It should not be a breaking change, because at every startup the state topics are passed to HA again via autodiscovery. If they change in the upcoming version, everything should run as usual, but on the new state topics. But I will test this extensively, so for now the merge in dev.

[thomasddn]

Updated the code so it will use volvoAAOS2mqtt as root topic. HA autodiscovery topics remain under homeassistant.

[Dielee]

Sorry for the delay. Will merge an test in the next few days!

[Dielee]

COVID got me, sorry. I will merge if I'm able to think again.

[thomasddn]

When would you have some time to have a look at this?

[thomasddn]

Hi @Dielee! Is there any chance that this pull request will be merged?

[Dielee]

Yeah, sorry. Maybe on Sunday. The frequent API changes and reallive requires a lot of time.

[github-actions[bot]]

This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days.

[thomasddn]

@Dielee Are you hesitant to merge this PR? Most applications work this way, e.g. What's Up Docker uses the same approach and I also do this in an add-on of my own.

[github-actions[bot]]

This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days.

[github-actions[bot]]

This PR was closed because it has been stalled for 10 days with no activity.