-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Remove oldmodel macro use #1735
Conversation
4c66df3
to
30ff2ac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this cleanup. I put some comments on some missed TODOs, I think those would be good to check before merging. Looks good in general.
I did look at most of this, but not everything. I don't see anything surprising though.
@@ -1364,24 +1363,18 @@ r_device acurite_th = { | |||
*/ | |||
static char *acurite_txr_output_fields[] = { | |||
"model", | |||
"subtype", // TODO: remove this | |||
"message_type", // TODO: remove this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one also has 'TODO: remove this'
@@ -75,7 +75,7 @@ static int bt_rain_decode(r_device *decoder, bitbuffer_t *bitbuffer) | |||
"battery", "Battery", DATA_STRING, battery ? "LOW" : "OK", | |||
"transmit", "Transmit", DATA_STRING, button ? "MANUAL" : "AUTO", // TODO: delete this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has a TODO: delete this, replaced by the 'button' value, I guess.
@@ -91,7 +91,6 @@ static char *output_fields[] = { | |||
"battery", | |||
"transmit", // TODO: delete this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be removed as well if it's removed above.
Shouldn't this be the occasion to also make sure all the "battery" string fields are replaced by "battery_ok" int fields? |
Thanks. Yes, since there is no oldmodel switch anymore there's no reason to have old battery fields. I'll make a second PR for a clean history without merge commits. |
30ff2ac
to
7ed4c4a
Compare
This removes the deprecated "oldmodel" workaround macro
_X()
.Regression checks pass, but if anyone can spare some time then please review.