ztalbot2000 / homebridge-cmd4

CMD4 Plugin for Homebridge - Supports ~All Accessory Types & now all Characteristics too
Apache License 2.0
149 stars 13 forks source link

[Feature Request] CMD4 to pass the attribute of config.json as it is. #97

Closed ghost closed 3 years ago

ghost commented 3 years ago

Hi John,

CMD4 executes state_cmd script with the attribute with UPPERCASE at first character. I want CMD4 to pass the attribute of config.json as it is.

For example:

config.json

{
   "type": "Lightbulb",
   "displayName": "DimLight",
   "on": "FALSE",
   "brightness": 0,
   "colorTemperature": 0,
   "name": "DimLight",
   "state_cmd": "/var/opt/infrasmacon/infrared_device.py"
},

CMD4 executes the following command line.

I emphasize "on" attribute of actual behavior is with UPPERCASE at first character.

Sufficient sleep, anatsuk1

ztalbot2000 commented 3 years ago

Hi,

The thing is, the sending is not wrong. Actually the config.json is. Cmd4 uppercases the first character in the config.json to match what it is supposed to be. It's a day one correction issue that I cannot get around. If you really want them to be the same, uppercase the first character of any Characteristic in your config.json file. That is what it should be and it will resolve your issue as well. Another reason I also cannot lower case the first letter is for characteristics like VOCDensity and there are many others. Believe me I'd love to generate a warning saying that you should use uppercase for the first character but that too would make a lot of people mad.

I hope the workaround I just gave you is acceptable.

ztalbot2000 commented 3 years ago

Just to go a little further, as I told you already CMD4 uppercases the characteristics in the config.json. This is true, but it does not uppercase CMD4 directives and "name" is one of those that is both a characteristic and a directive. Cmd4 does not like name in upper case. I tried it.

What I decided to do was make sure CMD4 responds to both. I will change all my test cases to upper case and documentation to upper case. Maybe in time I can drop lower case as it should never have been.

ttfn, John

On Sat, Feb 27, 2021 at 8:57 AM anatsuk1 notifications@github.com wrote:

Assigned #97 https://github.com/ztalbot2000/homebridge-cmd4/issues/97 to @ztalbot2000 https://github.com/ztalbot2000.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/ztalbot2000/homebridge-cmd4/issues/97#event-4385012991, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSBCXY4TRN2P5NR5ZYZU4TTBD24BANCNFSM4YJ63BOQ .

ghost commented 3 years ago

John,

Finally, I respect your decision. But I still have the different opinion. Anyway thanks for your work.

ztalbot2000 commented 3 years ago

Hi,

I just finished the submission of uppercase handling of CMD4 designations. It now handles both. The main reason I cannot change what is sent is that that would break everyones scripts as they already expect upperCase. I also don't actually keep the string you put in the config.json. It gets parsed into an array of integers that is easily indexed into two large tables of device, character characteristics and cached status's. There is no send routine for every characteristic. Well in version 1.0 there was but that was unmaintainable and suffered from an extreme number of typos. Those typos are what brought about the lower/upper case issue today.

This change should be released soon enough. I want to soak it a bit, though it does pass 8311 unit tests.

take care, John

On Sun, Feb 28, 2021 at 5:18 AM anatsuk1 notifications@github.com wrote:

John,

Finally, I respect your decision. But I still have the different opinion. Anyway thanks for your work.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ztalbot2000/homebridge-cmd4/issues/97#issuecomment-787428708, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSBCX3C2TXILNJPP6ANXCDTBIJ6ZANCNFSM4YJ63BOQ .