Skip to content
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

If changed sysname after the controller settings is done, the controller will report to old topic (using old name) #728

Closed
Grovkillen opened this issue Jan 15, 2018 · 13 comments
Labels
Category: Controller Related to interaction with other platforms Type: Bug Considered a bug
Milestone

Comments

@Grovkillen
Copy link
Member

https://www.letscontrolit.com/forum/viewtopic.php?f=6&t=4009

I just tried it and the log is not reporting any incoming commands using openHAB as MQTT controller.

@Grovkillen Grovkillen added Type: Bug Considered a bug Category: Controller Related to interaction with other platforms labels Jan 15, 2018
@Grovkillen Grovkillen added this to the 2.0.0 milestone Jan 15, 2018
@Grovkillen
Copy link
Member Author

Grovkillen commented Jan 15, 2018

OOOOOPS... I forgot that the topic is case sensitive. This is not a bug per say but I did find that the controller settings still used the sysname from when I first created the controller (it published "connection lost" to the old sysname topic)... That must be considered a bug?

@Grovkillen Grovkillen changed the title MQTT commands not accepted with later releases If changed sysname after the controller settings is done, the controller will report to old topic (using old name) Jan 15, 2018
@TD-er
Copy link
Member

TD-er commented Jan 17, 2018

Hmm, PiDome MQTT will also not work, judging the source code.
I will create a proper function to deal with it, just like Domoticz MQTT plugin does.

@TD-er
Copy link
Member

TD-er commented Jan 17, 2018

Second big Hmmm when looking at the code... I guess MQTT controllers can only work when being the first controller.
Maybe that should be split into another issue?

TD-er added a commit to TD-er/ESPEasy that referenced this issue Jan 17, 2018
See letscontrolit#728
Also looked into the very strange configuration settings lookup for MQTT settings. The way it was, the settings for the first MQTT controller were used even when the actual MQTT controller was inactive or using different settings.

Now the controller selected in the the plugin is being used and when no plugin setting is available (and also for P037 MQTT import) the first active MQTT controller will be used.
@TD-er
Copy link
Member

TD-er commented Jan 17, 2018

See Pull Request #737
Still have to fix the P037 MQTT Import.
That one allows for multiple MQTT, but with the current fix only the first enabled MQTT plugin will be used.

At least it is better than using settings from the 1st controller, even when that isn't a MQTT controller.

@psy0rz
Copy link
Member

psy0rz commented Jan 18, 2018

that was a known issue i think: Only the first controller can be MQTT. Its documented on the wiki.

psy0rz pushed a commit that referenced this issue Jan 18, 2018
See #728
Also looked into the very strange configuration settings lookup for MQTT settings. The way it was, the settings for the first MQTT controller were used even when the actual MQTT controller was inactive or using different settings.

Now the controller selected in the the plugin is being used and when no plugin setting is available (and also for P037 MQTT import) the first active MQTT controller will be used.
@TD-er
Copy link
Member

TD-er commented Jan 18, 2018

And now it has changed into "only one controller can be MQTT, or at least the first enabled MQTT will be used" ;)
Maybe, sometimes we can change it into a full implementation, but I am not sure it will be that useful to have multiple MQTT controllers.
At least it will require a lot more work and I am not really sure we have enough memory to spare.

@Grovkillen
Copy link
Member Author

The report is now on correct topic: %sysname%/status

But I have now realized that the bug also apply to the incoming command on %sysname%/cmd

It only respond to the old names topic for incoming command.

@Grovkillen
Copy link
Member Author

Regarding more MQTT controllers, I would suspect that someone could have the use of two different output formats. JSON (as Domoticz use) or plain text (as openHAB use).

But for now I really like that I'm able to have MQTT on other controller number than 1 since many people will have 1 occupied with some (perhaps HTTP) controller before wanting to test MQTT.

@TD-er
Copy link
Member

TD-er commented Jan 18, 2018

@Grovkillen
Please create another issue with all the MQTT related things not working like they should.
Then we have a good separation between things already fixed and "new" stuff :)

@Grovkillen
Copy link
Member Author

I will 👍

@Grovkillen
Copy link
Member Author

Just a quick question, did you change so we are now allowed to have MQTT on controller 1..3?

I added openHAB MQTT on controller 1, removed it, tried to add it to controller 2, it was greyed out.
I added openHAB MQTT on controller 1, replaced it with Domoticz HTTP (disabled), tried to add it to controller 2, it was greyed out.

@TD-er
Copy link
Member

TD-er commented Jan 18, 2018

Hmm, grey-out is something done in the webinterface. Haven't looked into that.
In the code it now either uses the set controller in the plugin, or when no plugin event is available, it searches for the first enabled MQTT controller when dealing with MQTT things.
It sounds like there was already some protection built in to disallow it.

@Grovkillen
Copy link
Member Author

Should I open an issue for that :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Controller Related to interaction with other platforms Type: Bug Considered a bug
Projects
None yet
Development

No branches or pull requests

3 participants