muart-group / esphome

ESPHome fork for development of the mitsubishi_uart component. Check out the documentation for more info:

https://muart-group.github.io/

10 stars 1 forks source link

Fix routing `SettingsSetRequest` from MHK #62

Closed jshearer closed 4 days ago

jshearer commented 4 days ago

What does this implement/fix?

Looks like a call to route_packet_ got dropped at some point, this adds it back.

Thanks @sle118 for the fix, I just made the PR :)

Tested on my SVZ-KP24NA with MHK2 thermostat

Types of changes

[x] Bugfix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] Other

Related issue or feature (if applicable): fixes https://github.com/muart-group/esphome/issues/61

Sammy1Am commented 4 days ago

Looks good, thanks for the PR (and thanks @sle118 for finding the issue)!

Sammy1Am commented 4 days ago

Dang it, I should have run the tests before merging. 😝 I figured, "It's just one line, surely it'll work."

There's an extra space before the semi-colon on the line you added that's making the CI tests fail. Would you mind updating the PR?

I don't know if I have to revert before you can update it since it's already "Merged"-- let me know if you have issues. I'm still a little new to the Github PR process.

Sammy1Am commented 4 days ago

~Ah, okay, maybe the PR can't be reopened, so I guess it'll have to be a separate PR. Again, apologies, should have run the tests.~

Nevermind, got it :)

sle118 commented 15 hours ago

Looks good, thanks for the PR (and thanks @sle118 for finding the issue)!

You can thank my significant other for identifying that there was a bug. All I had to do was to identify root cause.

Thank you for the fix!