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

Add support for Mi Desk Lamp #4887

Merged
merged 6 commits into from
Jan 15, 2019
Merged

Add support for Mi Desk Lamp #4887

merged 6 commits into from
Jan 15, 2019

Conversation

dh-harald
Copy link
Contributor

@dh-harald dh-harald commented Jan 11, 2019

Hi,

This is my initial commit for Mi Desk Lamp... It's WIP, no color temperature switch yet...
I'd like to add it for review, because I'm pretty sure, that you have coding standards, recommendations, etc, that I need to modify...

Status:

  • Add Mi Desk Lamp Module
  • Add support for rotary switch
  • Control brightness with rotary switch
  • Control color temperature with button press & rotary switch

Any reviews or suggestions are welcome

Copy link
Collaborator

@andrethomas2 andrethomas2 left a comment

Choose a reason for hiding this comment

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

The uint16_t rotary_debounce; could be uint8_t rotary_debounce; as it is unlikely that you would need a debounce of more than 255ms on a rotary encoder.

Also, @arendst will need to decide if the rotary support should be in a separate driver file or not so that it may be conditionally included or excluded for wider use cases where users just want to use a rotary encoder with any other project.

@Jason2866
Copy link
Collaborator

Would be cool to see a separate general rotary driver for Tasmota

@andrethomas
Copy link
Contributor

Is there any particular reason why "Control color temperature with button press & rotary switch" is not included in your initial PR ?

@dh-harald
Copy link
Contributor Author

dh-harald commented Jan 11, 2019

@andrethomas I've used uint16_t because the other debounce variables (button_debounce, pulse_counter_debounce, switch_debounce) are the same type.
In theory it could be higher, because I'm using interrupts for rotary change, then the loop just collects the change of the position... (Without interrupts, 10ms could be enough)

@dh-harald
Copy link
Contributor Author

@andrethomas Because I'm not a developer, I'd like to start the review process as soon as possible to taylor any coding style problems...
Abouth the "Control color temperature with button press & rotary switch" feature, I'm thinking about, how can I handle that... I need to figure out, how can I enable a "multipress" thing with the rotary switch, then hopefully, it won't turn off the LEDs, when the color temperature setting has been finished.

@Jason2866
Copy link
Collaborator

@dh-harald
Copy link
Contributor Author

@Jason2866 I used https://github.com/Torxgewinde/Desk-Lamp-Alternative-Firmware for the start, then (as I mentioned in the code), I used the Encoder library for decoding the rotary switch.

@arendst
Copy link
Owner

arendst commented Jan 11, 2019

In the future we might have to move the device specific code elsewhere but for now it can stay where it is.

To make things simpler I would like to get rid of the special rotary debounce code and use the key debounce instead. I do not expect deviations between the two.

Have a separate handler for the rotary switch is also a good idea for now as it allows changes to be kept to this file only.

For double press you might find some hints in the button.ino file.

@arendst arendst added the work in progress Action - Work in progress label Jan 11, 2019
Copy link
Owner

@arendst arendst left a comment

Choose a reason for hiding this comment

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

See my last comment

@dh-harald
Copy link
Contributor Author

@arendst Did you think about something like my last commit?

@arendst
Copy link
Owner

arendst commented Jan 11, 2019

Yes.

Copy link
Collaborator

@andrethomas2 andrethomas2 left a comment

Choose a reason for hiding this comment

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

Happy for the moment, so just need to see how @arendst wants to proceed.

I think the initial state of the rotary can also be pushed by home automation software, but may not be necessary.

@dh-harald dh-harald changed the title WIP: Add support for Mi Desk Lamp Add support for Mi Desk Lamp Jan 15, 2019
@dh-harald
Copy link
Contributor Author

I've added the color temperature change feature.
We should consider the following things:

  • KEY1 is hardcoded now to switch between color temp and brightness (maybe we could use a configuration)
  • Rotary button is working only with Mi Desk Lamp (if some other projects are using rotary switch, this could be changed)

Move rotary GPIO to non-user config for now.
@arendst arendst merged commit 6b7becb into arendst:development Jan 15, 2019
arendst added a commit that referenced this pull request Jan 15, 2019
6.4.1.9 20190115
 * Add support for Mi LED Desk Lamp with rotary switch (#4887)
 * Fix mDNS addService (#4938)
gemu2015 pushed a commit to gemu2015/Sonoff-Tasmota that referenced this pull request Jan 27, 2019
gemu2015 pushed a commit to gemu2015/Sonoff-Tasmota that referenced this pull request Jan 27, 2019
6.4.1.9 20190115
 * Add support for Mi LED Desk Lamp with rotary switch (arendst#4887)
 * Fix mDNS addService (arendst#4938)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work in progress Action - Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants