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

DDF for Third Reality, Inc smart button 3RSB22BZ #7897

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

louix
Copy link
Contributor

@louix louix commented Aug 22, 2024

Issue: #7863

@Smanar Smanar linked an issue Aug 23, 2024 that may be closed by this pull request
1 task
Copy link
Contributor

Hey @louix, thanks for your pull request!

Tip

Modified bundles can be downloaded here.
Relative expire date

DDB changes

Modified

  • third_reality/3RSB22BZ_smart_button.json : Smart Button 3RSB22BZ ✔️

Validation

Tip

Everything is fine !

🦆 Updated for commit 4a2cf71

@manup manup added this to the v2.29.0-beta milestone Sep 3, 2024
@manup manup changed the title DDF: 3RSB22BZ Third Reality, Inc smart button DDF for Third Reality, Inc smart button 3RSB22BZ Sep 5, 2024
@manup
Copy link
Member

manup commented Sep 5, 2024

Item.val={1: 1002, 2: 1004, 0: 1001, 255: 1003}[Number(ZclFrame.at(3))]

I don't think that will work in Javascript since object keys must be strings.

Here's an alternative approach (1):

const v = [1, 1002, 2, 1004, 0, 1001, 255, 1003];
const n = v.indexOf(ZclFrame.at(3));
if (n >= 0) Item.val = v[n + 1];

Or another one as one liner (2):

const n = ZclFrame.at(3); if (n < 3) { Item.val = [1001, 1002, 1004][n]; } else if (n == 255) { Item.val = 1003; }

Not pretty but should do the trick without string conversions :)

@Smanar
Copy link
Collaborator

Smanar commented Sep 5, 2024

Another one that have workied for him

n=Number(ZclFrame.at(3));t={'1': 1002, '2': 1004, '0': 1001, '255': 1003};if(n in t){Item.val=t[n]}

#7863 (comment)

Or just adding quote ?

@louix
Copy link
Contributor Author

louix commented Sep 5, 2024

Hey, apologies for the confusion.

It should be fine, JS engines internally coerces keys that are not string | Symbol via ToPropertyKey as part of the ECMAScript spec:

$ node --eval "console.log({1: true})"
{ '1': true }
$ node --eval "console.log({'1': 'hello'}[1])"
'hello'

A Map could also be used:

$ node --eval "console.log(new Map([[1, true]]))"
Map(1) { 1 => true }
$ node --eval "console.log(new Map([[1, 'hello'], [2, 'goodbye']]).get(1))"
hello

But it's subjectively a bit harder to read and probably a bit slower:

Item.val=new Map([[1, 1002], [2, 1004], [0, 1001], [255, 1003]]).get(Number(ZclFrame.at(3)))

Or you could be explicit about the conversions:

Item.val={'1': 1002, '2': 1004, '0': 1001, '255': 1003}[String(Number(ZclFrame.at(3)))]

The Number(ZclFrame.at(3) conversion is for 0 padding:

$ node --eval "console.log(Number('003'))"
3

Let me know what you prefer!

@Smanar
Copy link
Collaborator

Smanar commented Sep 5, 2024

lol, sometime we are searching a special function, sometime we found too much of them ^^.
On my side for a so simple convertion I prefer a "one liner", but you can use the one you want.

@manup
Copy link
Member

manup commented Sep 9, 2024

It should be fine, JS engines internally coerces keys that are not string | Symbol via ToPropertyKey as part of the ECMAScript spec:

Thanks I didn't know that. Could you test if this is also supported in DucktapeJS engine?
This is the engine used in deCONZ and supports only ECMAScript E5 and just a subset of newer versions https://duktape.org

@louix
Copy link
Contributor Author

louix commented Sep 9, 2024

Thanks Manuel.

Could you test if this is also supported in DucktapeJS engine?

Sure. They have a nice web REPL, and I am happy to report it works (version 20500).

I have tested the device integration with deCONZ, too.

All said and done, this is your codebase and the conventions are up to you, I'm happy to change to to something if it makes it more difficult for maintainers to reason about.

@manup
Copy link
Member

manup commented Sep 9, 2024

Cool nice that it works, then I see no problem for merging.

I like Web REPL, perhaps some time this should also be integrated into the UI for easier testing.

@manup manup merged commit 7e34c7e into dresden-elektronik:master Sep 9, 2024
1 check passed
Copy link
Contributor

github-actions bot commented Sep 9, 2024

This pull request is now merged. The new DDB files have been uploaded to the store.

DDB Files

Modified

  • third_reality/3RSB22BZ_smart_button.json : Smart Button 3RSB22BZ : with hash (d901e7ac7f)

🕧 Updated for commit 7e34c7e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Third Reality, Inc. Smart Button 3RSB22BZ
3 participants