SmartThingsCommunity / SmartThingsEdgeDrivers

Apache License 2.0
261 stars 447 forks source link

changed MA AVRs profile name to a more user friendly name #1367

Closed alon-tchelet closed 3 months ago

alon-tchelet commented 4 months ago

Trying to change the Device Name as this is not user friendly, as can be seen below: image

github-actions[bot] commented 4 months ago

Invitation URL: https://bestow-regional.api.smartthings.com/invite/Boj07N86pXlA

github-actions[bot] commented 4 months ago

Test Results

   57 files  ± 0    365 suites  +1   0s :stopwatch: ±0s 1 773 tests +12  1 773 :white_check_mark: +12  0 :zzz: ±0  0 :x: ±0  3 074 runs  +18  3 074 :white_check_mark: +18  0 :zzz: ±0  0 :x: ±0 

Results for commit 9f67e2b9. ± Comparison against base commit 90206d9f.

:recycle: This comment has been updated with latest results.

github-actions[bot] commented 4 months ago

Minimum allowed coverage is 90%

Generated by :monkey: cobertura-action against 9f67e2b95a8e9cedbf801f1d5bb945f892bf309f

alon-tchelet commented 4 months ago

@greens I don't know what it has to do with. But the only place in our code where the term "maX10" appears in is where I changed it. The function you pointed returns the user friendly name set by the user on the JBL app or web interface. By default it will be in the format seen under the column "DEVICE LABEL" (-<6-chars-of-mac>; e.g. "JBL MA9100HP-95...."). Hence, I believe "DEVICE NAME" is basing its value through the profile name. I don't know if it's done in the app code during/after the WWST certification process, but this is the only place this string is used.

greens commented 4 months ago

@alon-tchelet Ah. I see. Yes, that column comes from the profile name. But that "name" is not visible to the user in many places. Mainly in the spot you've pointed out here.

The issue, as Zach pointed out, is that we cannot delete profiles once they've been created. Renaming it as you've done here will count as removing an existing profile and creating a new one, which will cause the deploy of the driver to fail.

alon-tchelet commented 4 months ago

Hi @greens, I'm working for a company who works for Harman Luxury so I can't make these decisions myself. I'll raise it up with them.

It seems like this PR should not be merged, but if there's a way to just change the displayed value there that would be ideal. I can also direct the Harman Project leader to make these discussions with the relevant people (in case it's not you).

lelandblue commented 4 months ago

Hello @alon-tchelet The "Device Name" field that is appearing, in the screen shot you provided, is not the same name that the device will show as its default name when the device onboards to SmartThings.

I am going to encourage someone from SmartThings to reach out regarding this PR, as there might be an alternative solution to resolving the issue you are concerned with.

alon-tchelet commented 3 months ago

Closing this PR for now as Harman Luxury have decided not to make this change for now as it would require to apply the series to certification again