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

MQTT Light - error in publication on brightness setting #7810

Closed
EssendonPhoto opened this issue May 29, 2017 · 21 comments
Closed

MQTT Light - error in publication on brightness setting #7810

EssendonPhoto opened this issue May 29, 2017 · 21 comments

Comments

@EssendonPhoto
Copy link

Home Assistant release 0.45.1:

Python release : 3.4.2

Component/platform: MQTT Light

Description of problem:
I have a lighting system which I can control with MQTT. I am using the native hass mqtt broker. I can read the state, publish updates, etc.

I should also be able to control brightness. However when I publish to the brightness_command_topic hass also subsequently publishes a command_topic to turn the light ON (which has not been requested). This latter extraneous publication is interpreted by my lighting as an instruction to go to 100% brightness.

hass should not be publishing a command_topic - can this be removed please? Alternatively, turn the light on prior to setting the brightness

Expected:
When a brightness_command_topic is published then no command_topic should be published

Problem-relevant configuration.yaml entries and steps to reproduce:

light 9:
- platform: mqtt
  name: "Study - Ian"
  state_topic: "cbus/read/254/56/9/state"
  command_topic: "cbus/write/254/56/9/switch"
  brightness_state_topic: "cbus/read/254/56/9/level"
  brightness_command_topic: "cbus/write/254/56/9/ramp"
  brightness_scale: 100
  qos: 0
  payload_on: "ON"
  payload_off: "OFF"
  optimistic: false

Traceback (if applicable):


cbus/write/254/56/9/ramp 24
cbus/write/254/56/9/switch ON
cbus/read/254/56/9/state ON
cbus/read/254/56/9/level 24
cbus/read/254/56/9/state ON
cbus/read/254/56/9/level 100

The first line (write) is published by hass and is the command to set the brightness of light 9 to a level of 24 (% in this case). The second line (write) is an erroneous publication to set the light to be on. The next four lines (read) are the responses from the lighting system - confirming respectively setting the light to be on, setting brightness to be 24% (from the first pub), then setting the light to be on, and setting brightness to 100% (from the erroneous pub).

Additional info:

An alternative to not publishing the command_topic would be to publish it PRIOR to the brightness_state_topic - this would work for me

@moskovskiy82
Copy link

Yes can confirm this. I would agree to the solution that either it should be published BEFORE or not published at all.

@thoscut
Copy link
Contributor

thoscut commented Jun 30, 2017

In my case, a GPIO has to be set to PWM prior to setting the brightness. This could be done in one command if the brightness command could be templated with the brightness value and the switch_on could be disabled.
As the fix isn't that simple like moving the switch_on prior to all other command, I also considered writing a custom light component. Not sure about letting the existing component grow in its complexity.

@EssendonPhoto
Copy link
Author

OK, so then the best solution would be not to publish the switch_on command_topic at all when publishing a brightness_command_topic. This would have the additional benefit of consistency of approach.

I hope that one of the dev team is looking at this issue ... :)

@dnaphreak
Copy link

dnaphreak commented Jul 23, 2017

Having this same issue.

Home Assistant release 0.49.0

I agree not publishing the 'switch on' command_topic when brightness_command_topic is published would be best.

@thoscut
Copy link
Contributor

thoscut commented Jul 24, 2017

I think to not create regression errors, it should be clear that no other mqtt lights need exactly the current behavior. Perhaps this change should realized as an option to decide to not publish the switch_on prior to other commands in the configuration and to not alter the current default behavior.

@PianSom
Copy link

PianSom commented Jul 24, 2017

@tseel - good idea, though I wonder whether a better solution would be to allow for multiple mqtt publications for a brightness_command_topic (and possibly, for completeness, command_topic).

Now I think further, does anyone know whether this behaviour (i.e. superfluous publication of 'switch on' command_topic) is also exhibited for other topics: color_temp_command_topic, rgb_command_topic, white_value_command_topic, xy_command_topic?

@PianSom
Copy link

PianSom commented Oct 7, 2017

In case anyone is following this thread, a proposed solution is in beta over on #9330

@TD22057
Copy link
Contributor

TD22057 commented Oct 12, 2017

Please take a look at the fix I just submitted (details over in #9330) and if you can, grab the light/mqtt.py file and test it out. Based on my understanding of what's going on, I think it should fix your issue as it allows to you specify if the ON command should be sent before style (color, brightness, etc) commands, after style commands, or if the brightness topic should be used instead of ON (which is what I think you actually need in this case).

@mvillarejo
Copy link
Contributor

Hello,

in my case, setting: brightness_scale to 100 does not even work, it shows me a 1..255 range for the brightness.

any idea what's wrong?

thanks

@TD22057
Copy link
Contributor

TD22057 commented Oct 13, 2017

@mvillarejo Please post your configuration. If I use this, it seems to work fine. This is me clicking on the gui on button (which sends 100) and moving the brightness slider around. Note that by definition, the brightness value inside the class is always 0 to 255.

light:
  - platform: mqtt
    command_topic: "test/light/command"
    brightness_command_topic: "test/light/bright"
    color_temp_command_topic: "test/light/color"
    rgb_temp_command_topic: "test/light/rgb"
    on_command_type: "BRIGHTNESS"
    brightness_scale: 100

Then I get the following messages posted (the 100 value is when it turns on):

Subscribing to 'test/#'
test/light/bright              '100'
test/light/bright              '62'
test/light/bright              '30'
test/light/bright              '70'
test/light/bright              '100'

@mvillarejo
Copy link
Contributor

ok, my bad then, I just checked the message and you're right, value is between 0-100. I got confuse with the class value.

thanks.

fabaff pushed a commit that referenced this issue Oct 31, 2017
* Added ability to control when the on command is sent.

* Changed to allow only brightness command

* Code cleanup

* Added test cases for on command mode.

* Added addition test

* Changed brightness options to lower case.

* Fixed case of default value

* Remove default
@balloob balloob mentioned this issue Nov 2, 2017
@TD22057
Copy link
Contributor

TD22057 commented Dec 5, 2017

So has anyone been able to get the state topic to work with this? I'm trying to connect HA to an MQTT<->Insteon bridge that I'm writing and I can't get any of the HA MQTT lights (light, JSON light, and template light) to work with dimming. Basically I want something that sends the brightness (0=off, >0 = on), and receives the state of the light back as a brightness as well. With the mod I submitted for this issue, I can get the MQTT light to send the just the brightness just fine, but I can't get it to read the brightness from the state topic and turn the light on or off that way.

IMHO, there are some issues in the MQTT template and MQTT json light as well in that they say you can brightness in the templates but then they don't actually make that variable available in some cases. I may have to try another pull request - I think all 3 of these light classes should support brightness as an output and an input. I think the problem I have with the current system is that "ON" in HA means "turn on to the last brightness level that HA knows about" - notice that the slider doesn't move to 100% when on is clicked, it moves to whatever it was before. But the light may not know what that value is so it has no way to know to go to that value when ON is sent. That's why I think it makes a lot more sense for MQTT lights to just use a command and state topic that is 0->255 where 0 is off and 1-255 is the dimmer level. Then the state and command payloads are the same. At least that should be an option in the configuration.

Anyone else have any thoughts on this?

@PianSom
Copy link

PianSom commented Dec 5, 2017

Hi @TD22057

I am not sure I fully understand your question. But, yes, the state topic works as expected for me. I haven't tried out the MQTT template or MQTT json capabilities, since your corrections to the basic MQTT work sufficiently well for me. Are you asking for behaviour on the other two?

I am away from my house until tomorrow so can't double check, but my recollection is that if I change a light setting using a physical switch from OFF to, say, ON brightness 50% then the HA interface reflects this immediately. (I have a MQTT interface to my lighting system, so the new state gets published and HA picks it up.) I will check that this behaviour is indeed exhibited tomorrow.

(One cute side effect I hadn't expected from the code is that the the HA icon for lighting changes from grey (for OFF) to yellow (for ON 100%) to pale yellow (proportional to brightness).)

@PianSom
Copy link

PianSom commented Dec 6, 2017

Confirmed - a change (either ON/OFF or to a given brightness) in the physical switch issues a MQTT state change, which is reflected accurately in HA

@TD22057
Copy link
Contributor

TD22057 commented Dec 6, 2017

Thanks - I think the issue is that I'm trying to publish state topics that match the command topics. So if the set command is "128" to set brightness 50% and "0" to turn the light off, I want my state topic to be sent out the same way. It took awhile of beating my head against the wall but I finally got something that works by doing the following. Set the state topic and brightness state topic to the same, then use template magic to convert 0 to ON/OFF for the state payload and leave it as an integer for the brightness state payload. What's confusing (and I think a bug) about this is that I say make the OFF payload to be "0" but HA doesn't respect that on the inbound state side - it assumes that the inbound state is always ON/OFF.

  - platform: mqtt
    command_topic: "test/light/level"
    on_command_type: "brightness"
    payload_off: "0"
    state_topic: "test/light/state"
    state_value_template: '{% if value|int %}ON{% else %}OFF{% endif %}'
    brightness_command_topic: "test/light/level"
    brightness_state_topic: "test/light/state"

@PianSom
Copy link

PianSom commented Dec 6, 2017

I'm not entirely sure I understand you - surely the state topic should always be ON/OFF and the brightness state topic gives the brightness?

On my system a change in brightness results in BOTH a state of ON and brightness state publication.

@PianSom
Copy link

PianSom commented Dec 6, 2017

In case it helps, this is the mqtt interface for my system

@TD22057
Copy link
Contributor

TD22057 commented Dec 6, 2017

@PianSom I don't think so. If on_command_type: "brightness" is used, then the command topic publishes "OFF" or "[1-255]" as the ON/OFF topic. So since the light can be set to publish the brightness as the command, shouldn't it also accept the same as the state? Basically, my config says output:

off: "state/0"
on: "state/255"
dimming: "state/1-255"

So I think that the light should also accept those as the input for the the state topic. Otherwise the remote device has to be implemented to accept states in one format (integer brightness) and publish states in a totally different format (on/off). I just think the default behavior of the HA light should be symmetric - what you send to update is what you receive to show current state.

@PianSom
Copy link

PianSom commented Dec 6, 2017

@TD22057 Ah, ok. As you will have seen, that's not the way that my system works.

Personally, I think that separation of state and brightness (and eg colour temp, rgb etc) is the way to go.

But what do I know :)

@balloobbot
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.

Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍

@balloobbot
Copy link

This issue will be auto-closed because there hasn't been any activity for a few months. Feel free to open a new one if you still experience this problem 👍

@home-assistant home-assistant locked and limited conversation to collaborators Jul 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants