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 lywsd03mmc sensor #636

Merged
merged 11 commits into from
Dec 25, 2021
Merged

Add lywsd03mmc sensor #636

merged 11 commits into from
Dec 25, 2021

Conversation

cnelsonsic
Copy link
Contributor

@cnelsonsic cnelsonsic commented Dec 19, 2021

TODO:

  • Testing
  • Cleanup
  • Docs

Merging this will close #635

@theyosh
Copy link
Owner

theyosh commented Dec 19, 2021

Dude, that is way cool! I am unable to test this all, so I am very happy that you are trying to add this.

I made 2 remarks on your PR. Could you have a look at them?

@cnelsonsic
Copy link
Contributor Author

I have some changes I'll be pushing in a few moments, but I don't see your comments on this PR.

@theyosh
Copy link
Owner

theyosh commented Dec 19, 2021

Hmm, not sure why... but here is a screenshot made of this PR...
image

Copy link
Owner

@theyosh theyosh left a comment

Choose a reason for hiding this comment

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

Does this show up the two remarks?

Makefile Outdated Show resolved Hide resolved
hardware/sensor/lywsd03mmc_sensor.py Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@cnelsonsic
Copy link
Contributor Author

Also, the Makefile change isn't strictly required, I just found it useful while I was testing. I can break it out into another PR if you'd like to polish that elsewhere first.

@cnelsonsic cnelsonsic marked this pull request as ready for review December 20, 2021 02:04
@cnelsonsic cnelsonsic requested a review from theyosh December 21, 2021 20:41
@cnelsonsic
Copy link
Contributor Author

It turns out that the bluetooth scanning in TerrariumPI is really pretty nice, I apparently never really had it working in my setup.

This is pretty well finalized, can you review this again?

Makefile Outdated Show resolved Hide resolved
@theyosh
Copy link
Owner

theyosh commented Dec 22, 2021

And if you could also update the composer example in: https://github.com/theyosh/TerrariumPI/blob/main/docs/_tabs/install.md?plain=1#L79

And if the device looks the same as the 'original' MiTemp, then you could just copy https://github.com/theyosh/TerrariumPI/blob/main/docs/_hardware/mitemp_sensor.md?plain=1 to lywsd03mmc_sensor.md and replace the names.

Then the documentation is also up to date.

@theyosh
Copy link
Owner

theyosh commented Dec 23, 2021

Hi, do you have time to update the documentation? If not, I can do it also.... not a problem. Then I can merge this PR

@cnelsonsic
Copy link
Contributor Author

Yes, I'll be working on docs today.

@cnelsonsic cnelsonsic requested a review from theyosh December 25, 2021 04:36
@cnelsonsic
Copy link
Contributor Author

This device actually has a smiley face so I don't know if you want to update the photo as well.

@theyosh theyosh merged commit e646d15 into theyosh:main Dec 25, 2021
@theyosh
Copy link
Owner

theyosh commented Dec 25, 2021

Thanks a lot! And merry Christmas

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.

[FEATURE]: Add LYWSD03MMC Sensor Support.
2 participants