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

Add additionalParams documentation for zigbee library #315

Merged
merged 1 commit into from May 27, 2016

Conversation

Projects
None yet
4 participants
@varzac
Contributor

varzac commented May 20, 2016


**Parameters:**
- **Cluster**: The Cluster ID to read from
- **attributeId**: The ID of the attribute to read
- **additionalParams**: An optional map to specify additional parameters. Currently only the "mfgCode" key is supported to specify the manufacturing code for manufacturer specific attributes.

This comment has been minimized.

@tpmanley

tpmanley May 20, 2016

Contributor

Is there a precedent for identifying map keys like mfgCode? @unixbeast

This comment has been minimized.

@unixbeast

unixbeast May 26, 2016

Contributor

Not really. If the keys are going to be static, and the additionalParams map is well defined, I would like to see the key:value pair be in a table format of some kind. Something like:
+-------------+----------------+-----------------------------+
| key | value | Description |
+-------------+----------------+-----------------------------+

Something akin to this: http://docs.smartthings.com/en/latest/capabilities-reference.html#color-control

- Write a manufacturer specific attribute on the SmartThings multi-sensor
.. code-block:: groovy

zigbee.writeAttribute(0xFC02, 0x0, 0x20, 1, [mfgCode: 0x110A])

This comment has been minimized.

@tpmanley

tpmanley May 20, 2016

Contributor

Write the attribute as 0x0000 since it is a 16-bit value and I think it makes it easier to understand if it's the appropriate length for the data type.

@@ -203,6 +220,10 @@ Configure a ZigBee device's reporting properties. Refer to the *Configure Report

zigbee.configureReporting(0x0008, 0x0000, 0x20, 1, 3600, 0x01)

- Configure for a manufacturer specific report on the SmartThings multi-sensor

This comment has been minimized.

@tpmanley

tpmanley May 20, 2016

Contributor

The word "for" doesn't seem necessary

@varzac varzac force-pushed the varzac:zigbee-mfg-code branch 2 times, most recently from 747446b to 27c2dca May 20, 2016

@varzac

This comment has been minimized.

Contributor

varzac commented May 27, 2016

@tpmanley @unixbeast I changed how it is formatted and where the specifics of the additionalParams arg is discussed. Let me know what you think.

additionalParams
^^^^^^^^^^^^^^^^

Currently there are several zigbee methods that support an optional map parameter callded additionalParams. This is

This comment has been minimized.

@jimmyjames

jimmyjames May 27, 2016

Contributor

"called" misspelled

This comment has been minimized.

@tpmanley

tpmanley May 27, 2016

Contributor

zigbee -> ZigBee

This comment has been minimized.

@varzac

varzac May 27, 2016

Contributor

You mean you don't spell it "callded"? Weird.


Currently there are several zigbee methods that support an optional map parameter callded additionalParams. This is
inteded to be used to support future params without affecting backward compatability. Currently the following are
supported

This comment has been minimized.

@jimmyjames

jimmyjames May 27, 2016

Contributor

Add a ":" after "Currently the following are supported:"

@jimmyjames

This comment has been minimized.

Contributor

jimmyjames commented May 27, 2016

The formatting looks good. There is a misspelling that should get fixed up and another minor comment I had, but otherwise looks good.

^^^^^^^^^^^^^^^^

Currently there are several zigbee methods that support an optional map parameter callded additionalParams. This is
inteded to be used to support future params without affecting backward compatability. Currently the following are

This comment has been minimized.

@tpmanley

tpmanley May 27, 2016

Contributor

"intended" misspelled

supported

======= ======== ==============
key type Description

This comment has been minimized.

@tpmanley

tpmanley May 27, 2016

Contributor

Looks like extra space before type, description and integer. Also suggest consistent capitalization on column headers, probably start with a cap.

@varzac varzac force-pushed the varzac:zigbee-mfg-code branch from 27c2dca to 9ad8912 May 27, 2016

additionalParams
^^^^^^^^^^^^^^^^

Currently there are several ZigBee methods that support an optional map parameter called additionalParams. This is

This comment has been minimized.

@tpmanley

tpmanley May 27, 2016

Contributor

The word "Currently" seems unnecessary here.

This comment has been minimized.

@varzac

varzac May 27, 2016

Contributor

Good point.

@tpmanley

This comment has been minimized.

Contributor

tpmanley commented May 27, 2016

👍

@varzac varzac force-pushed the varzac:zigbee-mfg-code branch from 9ad8912 to ddd7660 May 27, 2016

@jimmyjames jimmyjames merged commit ed5dc6e into SmartThingsCommunity:master May 27, 2016

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