Skip to content

Conversation

@JerryYang01
Copy link

@JerryYang01 JerryYang01 commented Feb 6, 2026

Check all that apply

Type of Change

  • WWST Certification Request
    • If this is your first time contributing code:
      • I have reviewed the README.md file
      • I have reviewed the CODE_OF_CONDUCT.md file
      • I have signed the CLA
    • I plan on entering a WWST Certification Request or have entered a request through the WWST Certification console at developer.smartthings.com
  • Bug fix
  • New feature
  • Refactor

Checklist

  • I have performed a self-review of my code
  • I have commented my code in hard-to-understand areas
  • I have verified my changes by testing with a device or have communicated a plan for testing
  • I am adding new behavior, such as adding a sub-driver, and have added and run new unit tests to cover the new behavior

Description of Change

Summary of Completed Tests

A profile of PAD19 (dimmer switch).
The edge driver of PAD19 for WWST certification.
@CLAassistant
Copy link

CLAassistant commented Feb 6, 2026

CLA assistant check
All committers have signed the CLA.

@JerryYang01 JerryYang01 closed this Feb 6, 2026
@JerryYang01 JerryYang01 reopened this Feb 6, 2026
@greens greens changed the title Add pad19 dimmer driver WWSTCERT-10247 Add pad19 dimmer driver Feb 10, 2026
Copy link
Contributor

@greens greens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should just be adding a sub-driver to the base driver, not adding a new driver in a folder under the base driver.

Here's an example from another partner of how this looks: https://github.com/SmartThingsCommunity/SmartThingsEdgeDrivers/pull/2371/changes

@JerryYang01
Copy link
Author

JerryYang01 commented Feb 11, 2026 via email

@greens
Copy link
Contributor

greens commented Feb 11, 2026

@JerryYang01 based on the code you've included in this PR, it's not clear that you would gain anything by having a separate driver. Your device seems to behave according to the Z-Wave spec, and would be well covered by our base z-wave switch driver. Our base drivers are written so that most devices can integrate with just the addition of the device's fingerprint to the fingerprints.yml file. This is for our own east of maintainability, but also to make it easy for partners such as yourselves.

But also, if your intent was to have an all new driver, you would want it to be alongside zwave-switch in the directory structure, not under it.

@JerryYang01
Copy link
Author

JerryYang01 commented Feb 12, 2026 via email

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to Steven's feedback regarding the driver structure. Also be sure to remove the print() statements from this file.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants