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

changed get_os_temperature() to accommodate the AWR129, which has an extra decimal digit for >100˚ C #2446

Merged
merged 1 commit into from
Mar 23, 2023

Conversation

homeautomaton
Copy link
Contributor

As a BBQ thermometer, the AWR129 reads higher temperatures than a weather thermometer ever would. The hundreds digit appears, right in order, in the low half of the fifth byte of the message. Since I don't know whether the THR228N and other similar devices handled by the protocol decoder might have something besides "0" in that spot, I've made the code conditional, so that only the AWR129 will be affected.

I tested this in my oven above 200˚C, and the readings increased smoothly at each multiple of 100, rather than wrapping back to zero as it did before this change.

@zuckschwerdt
Copy link
Collaborator

Are you sure it is (message[5] & 0x0f) * 100.0F and not just (message[5] & 0x07) * 100.0F -- because the upper but is already the sign bit!
Then this can be the general case. No if (hundreds_digit) needed.

Otherwise in line 378 put this and don't change anything else:

float temp_c = get_os_temperature(msg);
if (sensor_id == ID_AWR129) {
    // The AWR129 BBQ thermometer has another digit (maybe it's always zero for other devices, and this conditional isn't needed?)
        temp_c += (message[5] & 0x07) * 100.0F;
}

        /* clang-format off */

@homeautomaton
Copy link
Contributor Author

the upper but is already the sign bit!

Thanks for the heads up. You'd think I could read the very next line of code! Yes, I'll change to 0x7 for the bit-and operation.

Then this can be the general case.

Then I'll remove the conditional, too.

@zuckschwerdt
Copy link
Collaborator

It seems there are THR228N test samples in #1525 -- if you want to check.

And you need to rebase this PR as the AWR129 is already merged.

@zuckschwerdt
Copy link
Collaborator

It's complicated to work from a changes "master". In the future us a "feature" branch. For now just force push your master here again.

@zuckschwerdt zuckschwerdt reopened this Mar 22, 2023
@homeautomaton
Copy link
Contributor Author

I was able to download and test with the THR228N samples from #1525. It is as we expect, for a positive temperature, the low half of byte 5 is all zeros. Hopefully the case with the many other Oregon Scientific products that share this protocol.

@homeautomaton
Copy link
Contributor Author

I had already pushed this:

146b40c

@zuckschwerdt zuckschwerdt merged commit 3264d01 into merbanan:master Mar 23, 2023
andrewjw pushed a commit to andrewjw/rtl_433 that referenced this pull request Sep 29, 2023
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.

None yet

2 participants