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 EcoWitt WH31B support by modifying WH31E #1732

Merged
merged 6 commits into from
Jun 16, 2021

Conversation

bitfliq
Copy link
Contributor

@bitfliq bitfliq commented Jun 3, 2021

Added samples here: merbanan/rtl_433_tests#394 , https://github.com/bitfliq/rtl_433_tests/tree/EcoWitt-WH31B .

This code worked for extracting data from WH31B - but I was unable to verify that it did not break the other devices.

Thanks for all of your hard work on this fantastic project.

@bitfliq bitfliq closed this Jun 3, 2021
@bitfliq
Copy link
Contributor Author

bitfliq commented Jun 3, 2021

Closing due to instability. Will reopen after fixes in place.

@zuckschwerdt
Copy link
Collaborator

Notes:

  • leave the crc-error distinction out, maybe add (%d) ... msg_type
  • leave long/short id out, use:
  • "model", "", DATA_COND, msg_type == 0x30, DATA_STRING, "AmbientWeather-WH31E",
    "model", "", DATA_COND, msg_type == 0x37, DATA_STRING, "AmbientWeather-WH31B",

Implementing feedback from @zuckschwerdt
@bitfliq
Copy link
Contributor Author

bitfliq commented Jun 6, 2021

Thanks @zuckschwerdt for friendly feedback. Instability seems resolved with your suggestions. Not confident I matched what you were looking for with crc-error distinction recommendation.

@bitfliq bitfliq reopened this Jun 6, 2021
@zuckschwerdt
Copy link
Collaborator

Looks mostly good, thanks! I meant to say: just change the error strings, e.g. "WH31E bad CRC" to "WH31E/WH31B bad CRC", there is no real need to have a switch on the msg_type there (just %d, like you did) -- shorter code is more legible :)

Perhaps we should keep CRC/SUM separate, so e.g. fprintf(stderr, "%s: WH31E/WH31B (%s) bad CRC\n", __func__, msg_type);

@bitfliq
Copy link
Contributor Author

bitfliq commented Jun 11, 2021

@zuckschwerdt Could you please take another look?

@Mindavi Mindavi merged commit aa14b6d into merbanan:master Jun 16, 2021
Mindavi pushed a commit to merbanan/rtl_433_tests that referenced this pull request Jun 16, 2021
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.

3 participants