[Bug] Zones with single quotes in names throw errors

[michaelroper]

Sorry, I don't have all the details for a good bug report, but just wanted to flag that I found that when the named zones on my MyAir controller had single quotes in their names, eg Michael's Room, something along the line was having issues with those zones specifically, as they would work sometimes, but more often show as Not Responding in Homekit, and have errors like this in Homebridge:

[12/6/2023, 7:27:48 PM] [Cmd4] Setting Michael's Room Zone RotationSpeed 5
[12/6/2023, 7:27:48 PM] [Cmd4] *1* error(s) were encountered for "Michael's Room Zone" getValue. Last error found Getting: "RotationSpeed". Perhaps you should run in debug mode to find out what the problem might be.
[12/6/2023, 7:27:58 PM] [Cmd4] Setting Michael's Room Zone On 1
[12/6/2023, 7:27:58 PM] [Cmd4] *1* error(s) were encountered for "Michael's Room Zone" getValue. Last error found Getting: "On". Perhaps you should run in debug mode to find out what the problem might be.
[12/6/2023, 7:28:08 PM] [Cmd4] Setting Michael's Room Zone RotationSpeed 74
[12/6/2023, 7:28:08 PM] [Cmd4] *1* error(s) were encountered for "Michael's Room Zone" getValue. Last error found Getting: "RotationSpeed". Perhaps you should run in debug mode to find out what the problem might be.

When I noticed that happening, I removed the quotes from my zone names, and recreated my config, and all seems fine so far. Maybe some strings are not being escaped properly, either in this plugin, or in Cmd4? I can try and provide more detail if it helps, but just wanted to flag it in case anyone else ran into similar issues.

[mitch7391]

Hey @michaelroper, cheers for submitting this. You would be 100% correct that the ’ would be causing issues, it would more than likely be effecting the parsing in our bash scripts off the top of my head. I know we have some notes on naming conventions in the Wiki, but it actually doesn’t mention the use of ’ from memory.

@uswong is this something we can get around at all to stop it catching out others? Or should we have it down as more of a warning to users?

[ztalbot2000]

Hi Guys,

It is not a CMD4 thing as I just pass the parameters to a script, which could be in any language, so I cannot parse the parameters as it is impossible to handle every language out there. Your bash script unfortunately would have to and it's a nasty thing to do as places you may or may not want to escape special characters. In Cmd4 I might have noted that you should not put special characters in any names as the scripts you write must handle them. You're not just dealing with single apostrophes either. Imagine what would happen with an ampersand or double quote, just to name a few. My suggestion is to not allow these characters.

Ttyl, John

On Wed, Dec 13, 2023 at 5:57 AM Mitch Williams @.***> wrote:

Hey @michaelroper https://github.com/michaelroper, cheers for submitting this. You would be 100% correct that the ’ would be causing issues, it would more than likely be effecting the parsing in our bash scripts off the top of my head. I know we have some notes on naming conventions in the Wiki, but it actually doesn’t mention the use of ’ from memory.

@uswong https://github.com/uswong is this something we can get around at all to stop it catching out others? Or should we have it down as more of a warning to users?

— Reply to this email directly, view it on GitHub https://github.com/mitch7391/homebridge-cmd4-AdvantageAir/issues/89#issuecomment-1853695365, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSBCXY47K27KZSNMUYTTSDYJGCYHAVCNFSM6AAAAABASX6KQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJTGY4TKMZWGU . You are receiving this because you are subscribed to this thread.Message ID: @.*** com>

[uswong]

I agree with John that the single apostrophe in a name is a nasty thing to handle in bash script. We use both single and double apostrophes in the script and hence if a name uses an apostrophe, it might confuse the script. As such, I totally agree that we should avoid using apostrophe as part of the name.

[michaelroper]

Thanks guys - would it be feasible to just strip any quotes from zone names during the config creation process? I think relying on users finding documentation that mentions not to use quotes in the names might not be as clear (eg I'd read through the repo readme file, but did not realise that wiki page existed!).

[mitch7391]

@michaelroper the ‘ConfigCreator’ is also a bash script, so it would also face the same challenges unfortunately.

I do agree that the information buried in a Wiki or our extensive README, might be a bit overwhelming or easy to miss, I think if I at least make it a note in the installation instructions, then surely that should not be missed. I would hope all new users are following the installation instructions at the very least haha…

[uswong]

Hey guys,

I do prefer to have a more wholistic solution to this single apostrophe issue. It is more than often that a zone's name or a light's name have a single apostrophe in it, for example "Ollie's Playroom" or "Hazel's Room Light".

I did a bit of testing and have some success. It requires very minor changes in both CMD4 and AdvAir.sh bash script.

All need to be modified in CMD4 are 2 lines in Cmd4PriorityPollingQueue.js:

Based on CMD4 version 7.0.1, change line 611 from:

let cmd = accessory.state_cmd_prefix + accessory.state_cmd + " Set '" + accessory.displayName + "' '" + characteristicString  + "' '" + value  + "'" + accessory.state_cmd_suffix;

to

let cmd = accessory.state_cmd_prefix + accessory.state_cmd + ' Set "' + accessory.displayName + '" "' + characteristicString  + '" "' + value  + '"' + accessory.state_cmd_suffix;

and change line 429 from:

let cmd = self.state_cmd_prefix + self.state_cmd + " Get '" + self.displayName + "' '" + characteristicString  + "'" + self.state_cmd_suffix;

to

let cmd = self.state_cmd_prefix + self.state_cmd + ' Get "' + self.displayName + '" "' + characteristicString  + '"' + self.state_cmd_suffix;

Basically, swapping double apostrophes with single apostrophe and vice versa so that if the self.displayName has a single apostrophe as part of the name, the whole displayName parameter will be passed to the bash script as one single parameter with the apostophe in it.

John, do you think the above is reasonable? If so, will you please help to get that sorted?

For AdvAir.sh bash script, the only line needs to be modified is to pick up the name of the light or thing from the displayName. I will get that sorted and will do a PR after I have fully tested it.

No modifications to ConfigCreator.sh is technically necessary but can be modified to remove the redundancy as a result of the modification done to AdvAir.sh.

[mitch7391]

Oh nicely done @uswong!

[uswong]

It is not a CMD4 thing as I just pass the parameters to a script, which could be in any language, so I cannot parse the parameters as it is impossible to handle every language out there.

Hey John, It does turn up to be the CMD4 thing because when the dispalyName has a single apostrophe, the displayName parameter passed to the script was mutilated.

The two examples below illustrate the issue:

First example is with a displayName (Bed 3 Zone) without apostrophe and works beautifully.

 '/var/lib/homebridge/node_modules/homebridge-cmd4-advantageair/AdvAir.sh' Set 'Bed 3 Zone' 'On' '1' z08 192.168.1.3:2025

The second example has its displayName (Meal's Zone) with a single apostrophe:

 '/var/lib/homebridge/node_modules/homebridge-cmd4-advantageair/AdvAir.sh' Set 'Meal's Zone' 'On' '1' z03 192.168.1.3:2025

In this second example, the second argument which is supposed to be Meal's Zone, is now Meal, and third argument which is supposed to be the characteristic On is now s Zone. Hence the bash script operation failed and returned an error.

The solution: pass on the displayName parameter in double apostrophe.

 '/var/lib/homebridge/node_modules/homebridge-cmd4-advantageair/AdvAir.sh' Set "Meal's Zone" "On" "1" z03 192.168.1.3:2025

[mitch7391]

Very good work here @uswong!

@ztalbot2000 do you think this is something you could implement in Cmd4?

[ztalbot2000]

Hi,

As I mentioned. This cannot be a part of Cmd4 because your assuming this is being passed to a bash script. It could be Python, a C++ binary or any other multitude of languages out there and the problem is Cmd4 does not know how that language parses parameters passed to it, so Cmd4 leaves that up to the person who writes the tool that processes the state_cmd. I know AdvAir passes this to the device as a database query, which databases are even more fussy with quotes, but that is up to AdvAir on how they want to handle this issue.

Ttyl, John

On Thu, Dec 14, 2023 at 4:01 AM Ung Sing Wong @.***> wrote:

Hey guys,

I do prefer to have a more wholistic solution to this single apostrophe issue. It is more than often that a zone's name or a light's name have a single apostrophe in it, for example "Ollie's Playroom" or "Hazel's Room Light".

I did a bit of testing and have some success. It requires very minor changes in both CMD4 and AdvAir.sh bash script.

All need to be modified in CMD4 are 2 lines in Cmd4PriorityPollingQueue.js:

Based on CMD4 version 7.0.1, change line 611 from:

let cmd = accessory.state_cmd_prefix + accessory.state_cmd + " Set '" + accessory.displayName + "' '" + characteristicString + "' '" + value + "'" + accessory.state_cmd_suffix;

to

let cmd = accessory.state_cmd_prefix + accessory.state_cmd + ' Set "' + accessory.displayName + '" "' + characteristicString + '" "' + value + '"' + accessory.state_cmd_suffix;

and change line 429 from:

let cmd = self.state_cmd_prefix + self.state_cmd + " Get '" + self.displayName + "' '" + characteristicString + "'" + self.state_cmd_suffix;

to

let cmd = self.state_cmd_prefix + self.state_cmd + ' Get "' + self.displayName + '" "' + characteristicString + '"' + self.state_cmd_suffix;

Basically, swapping double apostrophes with single apostrophe and vice versa so that if the self.displayName has a single apostrophe as part of the name, the whole displayName parameter will be passed to the bash script as one single parameter with the apostophe in it.

John, do you think the above is reasonable? If so, will you please help to get that sorted?

For AdvAir.sh bash script, the only line needs to be modified is to pick up the name of the light or thing from the displayName. I will get that sorted and will do a PR after I have fully tested it.

— Reply to this email directly, view it on GitHub https://github.com/mitch7391/homebridge-cmd4-AdvantageAir/issues/89#issuecomment-1855442807, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSBCX2OA45PBSGL72NGRV3YJK55NAVCNFSM6AAAAABASX6KQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJVGQ2DEOBQG4 . You are receiving this because you commented.Message ID: @.***>

[ztalbot2000]

Hi Sing,

If you swap single quotes for double quotes then the name cannot have a $ in it as this would mean an environmental variable and be undefined. Other special characters would also be interpreted differently and depending on what the language is, open up a big world of hurt. This is why single quotes are used in the first place.

Ttyl, John

On Thu, Dec 14, 2023 at 4:01 AM Ung Sing Wong @.***> wrote:

Hey guys,

I do prefer to have a more wholistic solution to this single apostrophe issue. It is more than often that a zone's name or a light's name have a single apostrophe in it, for example "Ollie's Playroom" or "Hazel's Room Light".

I did a bit of testing and have some success. It requires very minor changes in both CMD4 and AdvAir.sh bash script.

All need to be modified in CMD4 are 2 lines in Cmd4PriorityPollingQueue.js:

Based on CMD4 version 7.0.1, change line 611 from:

let cmd = accessory.state_cmd_prefix + accessory.state_cmd + " Set '" + accessory.displayName + "' '" + characteristicString + "' '" + value + "'" + accessory.state_cmd_suffix;

to

let cmd = accessory.state_cmd_prefix + accessory.state_cmd + ' Set "' + accessory.displayName + '" "' + characteristicString + '" "' + value + '"' + accessory.state_cmd_suffix;

and change line 429 from:

let cmd = self.state_cmd_prefix + self.state_cmd + " Get '" + self.displayName + "' '" + characteristicString + "'" + self.state_cmd_suffix;

to

let cmd = self.state_cmd_prefix + self.state_cmd + ' Get "' + self.displayName + '" "' + characteristicString + '"' + self.state_cmd_suffix;

Basically, swapping double apostrophes with single apostrophe and vice versa so that if the self.displayName has a single apostrophe as part of the name, the whole displayName parameter will be passed to the bash script as one single parameter with the apostophe in it.

John, do you think the above is reasonable? If so, will you please help to get that sorted?

For AdvAir.sh bash script, the only line needs to be modified is to pick up the name of the light or thing from the displayName. I will get that sorted and will do a PR after I have fully tested it.

— Reply to this email directly, view it on GitHub https://github.com/mitch7391/homebridge-cmd4-AdvantageAir/issues/89#issuecomment-1855442807, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSBCX2OA45PBSGL72NGRV3YJK55NAVCNFSM6AAAAABASX6KQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJVGQ2DEOBQG4 . You are receiving this because you commented.Message ID: @.***>

[uswong]

Hi John,

I do understand your concern if a name is defined as an environmental variable with a $ in it but for AdvantageAir, every name or displayName is defined as a string and not an environmental variable.

Are there other CMD4 users use environmental variable as device's name?

If so, would you please do a special case for AdvantageAir users? I have tested successfully adding the following lines after line 429 of Cmd4PriorityPollingQueue.js:

      if ( self.state_cmd.match( /AdvAir.sh'$/ ) )
      {
         cmd = self.state_cmd_prefix + self.state_cmd + ' Get "' + self.displayName + '" "' + characteristicString  + '"' + self.state_cmd_suffix;
      } 

And adding the following lines after line 611

  if ( accessory.state_cmd.match( /AdvAir.sh'$/ ) )
  {
     cmd = accessory.state_cmd_prefix + accessory.state_cmd + ' Set "' + accessory.displayName + '" "' + characteristicString  + '" "' + value  + '"' + accessory.state_cmd_suffix;
  }

[ztalbot2000]

Hi Sing,

Possible. Not something I'd want to do for everyone who asks, though you are special ;-)

I don't know how many people use Cmd4 with funny character names. It's not something I'd change just to find out.

Have you ever thought of copying Cmd4 into your own repo? It might solve some of your own distribution problems as well.

I'll think about the change you want over the next few days. Most likely I'll do it, just need some time.

Ttyl, John

On Wed, Dec 20, 2023 at 4:10 AM Ung Sing Wong @.***> wrote:

Hi John,

I do understand your concern if a name is defined as an environmental variable with a $ in it but for AdvantageAir, every name or displayName is defined as a string and not an environmental variable.

Are there other CMD4 users use environmental variable as device's name?

If so, would you please do a special case for AdvantageAir users? I have tested successfully adding the following lines after line 429 of Cmd4PriorityPollingQueue.js:

  if ( self.state_cmd.match( /AdvAir.sh'$/ ) )
  {
     cmd = self.state_cmd_prefix + self.state_cmd + ' Get "' + self.displayName + '" "' + characteristicString  + '"' + self.state_cmd_suffix;
  }

And adding the following lines after line 611

if ( accessory.state_cmd.match( /AdvAir.sh'$/ ) ) { cmd = accessory.state_cmd_prefix + accessory.state_cmd + ' Set "' + accessory.displayName + '" "' + characteristicString + '" "' + value + '"' + accessory.state_cmd_suffix; }

— Reply to this email directly, view it on GitHub https://github.com/mitch7391/homebridge-cmd4-AdvantageAir/issues/89#issuecomment-1864111385, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSBCX4VFOU7YOHNK2Y4KT3YKKTQPAVCNFSM6AAAAABASX6KQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRUGEYTCMZYGU . You are receiving this because you were mentioned.Message ID: @.***>

[uswong]

Thanks John. Much appreciated.

Mitch and I did talk about copying CMD4 into our own repo a while ago but we were not familiar with how to go about to make it work, hence dropped the idea. Since you mentioned it now, I might take another look at it in the new year.

Best regards, Ung Sing

---

From: John Talbot @.> Sent: Wednesday, 20 December 2023 20:56 To: mitch7391/homebridge-cmd4-AdvantageAir @.> Cc: Ung Sing Wong @.>; Mention @.> Subject: Re: [mitch7391/homebridge-cmd4-AdvantageAir] [Bug] Zones with single quotes in names throw errors (Issue #89)

Hi Sing,

Possible. Not something I'd want to do for everyone who asks, though you are special ;-)

I don't know how many people use Cmd4 with funny character names. It's not something I'd change just to find out.

Have you ever thought of copying Cmd4 into your own repo? It might solve some of your own distribution problems as well.

I'll think about the change you want over the next few days. Most likely I'll do it, just need some time.

Ttyl, John

On Wed, Dec 20, 2023 at 4:10 AM Ung Sing Wong @.***> wrote:

Hi John,

I do understand your concern if a name is defined as an environmental variable with a $ in it but for AdvantageAir, every name or displayName is defined as a string and not an environmental variable.

Are there other CMD4 users use environmental variable as device's name?

If so, would you please do a special case for AdvantageAir users? I have tested successfully adding the following lines after line 429 of Cmd4PriorityPollingQueue.js:

if ( self.state_cmd.match( /AdvAir.sh'$/ ) ) { cmd = self.state_cmd_prefix + self.state_cmd + ' Get "' + self.displayName + '" "' + characteristicString + '"' + self.state_cmd_suffix; }

And adding the following lines after line 611

if ( accessory.state_cmd.match( /AdvAir.sh'$/ ) ) { cmd = accessory.state_cmd_prefix + accessory.state_cmd + ' Set "' + accessory.displayName + '" "' + characteristicString + '" "' + value + '"' + accessory.state_cmd_suffix; }

— Reply to this email directly, view it on GitHub https://github.com/mitch7391/homebridge-cmd4-AdvantageAir/issues/89#issuecomment-1864111385, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSBCX4VFOU7YOHNK2Y4KT3YKKTQPAVCNFSM6AAAAABASX6KQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRUGEYTCMZYGU . You are receiving this because you were mentioned.Message ID: @.***>

— Reply to this email directly, view it on GitHubhttps://github.com/mitch7391/homebridge-cmd4-AdvantageAir/issues/89#issuecomment-1864428506, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AXAO6PIE2GYOZ3EYYC4CCHDYKLN7FAVCNFSM6AAAAABASX6KQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRUGQZDQNJQGY. You are receiving this because you were mentioned.Message ID: @.***>

[ztalbot2000]

Merry Christmas all,

I just published homebridge-cmd4 version 7.0.2 which changes double quotes to single quotes for the accessory name. What you sent me put double quotes around the characteristic name and values, which is not required. I also added four new unit tests that prove it all works and does not affect other users of Cmd4.

Enjoy, John Talbot

On Wed, Dec 20, 2023 at 9:53 PM Ung Sing Wong @.***> wrote:

Thanks John. Much appreciated.

Mitch and I did talk about copying CMD4 into our own repo a while ago but we were not familiar with how to go about to make it work, hence dropped the idea. Since you mentioned it now, I might take another look at it in the new year.

Best regards, Ung Sing

---

From: John Talbot @.> Sent: Wednesday, 20 December 2023 20:56 To: mitch7391/homebridge-cmd4-AdvantageAir @.> Cc: Ung Sing Wong @.>; Mention @.> Subject: Re: [mitch7391/homebridge-cmd4-AdvantageAir] [Bug] Zones with single quotes in names throw errors (Issue #89)

Hi Sing,

Possible. Not something I'd want to do for everyone who asks, though you are special ;-)

I don't know how many people use Cmd4 with funny character names. It's not something I'd change just to find out.

Have you ever thought of copying Cmd4 into your own repo? It might solve some of your own distribution problems as well.

I'll think about the change you want over the next few days. Most likely I'll do it, just need some time.

Ttyl, John

On Wed, Dec 20, 2023 at 4:10 AM Ung Sing Wong @.***> wrote:

Hi John,

I do understand your concern if a name is defined as an environmental variable with a $ in it but for AdvantageAir, every name or displayName is defined as a string and not an environmental variable.

Are there other CMD4 users use environmental variable as device's name?

If so, would you please do a special case for AdvantageAir users? I have tested successfully adding the following lines after line 429 of Cmd4PriorityPollingQueue.js:

if ( self.state_cmd.match( /AdvAir.sh'$/ ) ) { cmd = self.state_cmd_prefix + self.state_cmd + ' Get "' + self.displayName + '" "' + characteristicString + '"' + self.state_cmd_suffix; }

And adding the following lines after line 611

if ( accessory.state_cmd.match( /AdvAir.sh'$/ ) ) { cmd = accessory.state_cmd_prefix + accessory.state_cmd + ' Set "' + accessory.displayName + '" "' + characteristicString + '" "' + value + '"'

• accessory.state_cmd_suffix; }

— Reply to this email directly, view it on GitHub < https://github.com/mitch7391/homebridge-cmd4-AdvantageAir/issues/89#issuecomment-1864111385>,

or unsubscribe < https://github.com/notifications/unsubscribe-auth/ABSBCX4VFOU7YOHNK2Y4KT3YKKTQPAVCNFSM6AAAAABASX6KQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRUGEYTCMZYGU>

. You are receiving this because you were mentioned.Message ID: @.***>

— Reply to this email directly, view it on GitHub< https://github.com/mitch7391/homebridge-cmd4-AdvantageAir/issues/89#issuecomment-1864428506>, or unsubscribe< https://github.com/notifications/unsubscribe-auth/AXAO6PIE2GYOZ3EYYC4CCHDYKLN7FAVCNFSM6AAAAABASX6KQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRUGQZDQNJQGY>.

You are receiving this because you were mentioned.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/mitch7391/homebridge-cmd4-AdvantageAir/issues/89#issuecomment-1865404151, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSBCX6KVEOVEQCF22CYOYDYKOQENAVCNFSM6AAAAABASX6KQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRVGQYDIMJVGE . You are receiving this because you were mentioned.Message ID: @.***>

[uswong]

Merry Christmas!

I just published homebridge-cmd4 version 7.0.2 which changes double quotes to single quotes for the accessory name.

Many thanks to John (@ztalbot2000 ) for the beautiful Christmas present to the AdvantageAir users' community. I installed CMD4 version 7.0.2, and it works brilliantly! Yes, even for accessory with single quote as part of the accessory's name likeOllie's Playroom Zone.

Best regards, Ung Sing

[mitch7391]

Resolved with homebridge-cmd4 v7.0.2 and v3.11.0 of this plug-in that has just bee rolled out now.

[michaelroper]

Thanks all for picking this one up and closing it out! 🙌