Add mitsubishi_itp Component #7289
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #7289 +/- ##
==========================================
+ Coverage 53.70% 53.96% +0.25%
==========================================
Files 50 50
Lines 9408 9660 +252
Branches 1654 1705 +51
==========================================
+ Hits 5053 5213 +160
- Misses 4056 4123 +67
- Partials 299 324 +25 ☔ View full report in Codecov by Sentry. |
|
I'm interested in the MHK2 part. Is that the one which normally plugs in via the different CNRF connector? And we then forward the communication from it? It would also be useful to be able to proxy for a MelCloud WiFi unit which normally connects to CN105; can that be supported? |
The MHK2 also plugs in via CN105 (at least in the NA market). If CNRF follows the same IT Protocol, it might very well work; should be easy enough to check by snooping on the UART and seeing if it conforms to the protocol standard.
This will work-ish, but with varying degrees of accuracy. We made the decision in the process to not officially support sitting in between a heat pump and a "smart" unit, as said "smart" unit will often make decisions that throws off our ability to track and manage state properly. As an example, something we're still working through is auto mode. As it currently stands, most units that provide a dual setpoint auto mode don't actually send both setpoints to the unit - they instead only send the active command and then manage it via internal logic. In certain cases, this desync can become rather significant. Additionally, for the most part, we've been focusing on the NA units (since that's what Sam and I have easy access to), with some smattering of compatibility for the EU and NZ/AUS units. There are slight variances on the IT protocol for each market, so some components may or may not work properly initially. See also the JP units which we haven't tested at all and include significant protocol variance. |
When you say "proxy for", do you mean "stand in place of"? Because that's largely the goal; that an ESPHome device with this component could replace a Kumo Cloud or similar WiFi unit. We're just avoiding supporting the ESPHome device being connected to a Kumo Cloud since it's weirdly complicated (see above). As for MelClo 8000 ud specifically, like @KazWolfe said it depends a bit on if the protocol is different for that market. From what I've seen the protocols are all relatively compatible though, so even if it doesn't entirely work out of the box it should be possible with some small switches or changes to the packets (but we'd need someone with the equipment to sniff/test). |
No, I mean sending packets onwards to (and from) it. I've just had a dual Ecodan cascade system installed, so three FTC6 controllers to plug devices into. I had started to look at https://github.com/gekkekoe/esphome-ecodan-hp The installer will provide ongoing service and monitor it via MelCloud, so I wanted to connect ESPHome without having to ditch MelCloud completely (but only really using MelCloud for monitoring not control). |
|
Ah, okay. If you're only using MelCloud for monitoring they should co-exist fairly well (as long as MelCloud doesn't decide to do anything proactively :P ). The MelCloud would act similarly to the MHK2 thermostat as far as this device is concerned, though the No support in this component yet for Ecodan, but if you're willing to help test, I'll think about if there's a good way to add support into this component (the basic pieces are the same, but because the protocol is different we might have to add an additional layer of abstraction in there or something). It'll also be easier to manage once this PR is done, though I've got another feature I kinda want added so I might have to tackle all that anyway soon. |
This is not exactly accurate, and I hope you implement it correctly. AUTO is built into the units, it has a 2C spread - this is useful for those of us who are ok with a larger fluctuation in the temps but want them to stay in a rage. From all my testing there is no way to change the 2C number. From the specs In this mode, the unit will run on HEAT or COOL depending on the room and set temperatures. Once the temperature is reached, the outdoor unit will slow down and eventually stop. The indoor fan will effectively stop when the indoor unit cools down to avoid a draught. When the temperature in the room falls during winter, heating will resume. If the room temperature is 2ºC above the set temperature in winter, the unit will automatically switch to COOL to maintain temperature. During summer, the unit running on AUTO mode will automatically switch to cooling to maintain temperature. AUTO is my Prefered mode when I am home and working - it keeps the house in a nice zone 22C (68-75ish). Dual Set Point (H/L) is enabled in the device controlling the split unit NOT THE UNIT. So Kumo, Thermostat, etc.... they handle this logic and only send over the currently desired mode to the split unit. This is the same with the auto dry mode in the Kumo adapters - the kumo adapter is doing all the work to enable auto dry. I helped make the first of these back years ago and have been chipping in work on one of the most commonly used libs (that I use in my house). The correct way to implement this is to enable BOTH AUTO and DUAL (H/L) because...why not, but do not confuse AUTO with Dual Set Point, please; if you want to be the one ring to rule them all; do this completely. Because I have written his code 3 times over the last 6 years or so and am sick of it! HAHAHA! My way of implementing H/L is to send in a temp and a spread, but HA doesnt like this OOTB, but I dont care, because I automate it and I'm reasonably technical. Example: 70F and 10F this means it will be in fan only mode/auto from 60F-80F and then at 80F it turns on cool and at 60F it turns on heat. This is great for traveling in areas like mine with high D/N temp swings (it was 85 and 55 here yesterday!). I got lazy and didn't feel like implementing the dual set point in HA because it just was really silly to me how hard it was to implement both single and dual...leaving that for others to do :) . |
At present, we are implementing AUTO correctly to my knowledge (since all we do is send the "auto" command to the unit). This still has some slight quirks, like the fact that an MHK2 will see the "auto" command and decide that the user really meant dual setpoint mode and take over. I think we do have plans to support dual setpoint mode, but we need to figure out how best to calculate and resolve temperature commands accordingly. There's also a bunch of very interesting questions about how to make auto vs dual setpoint vs everything else actually work with the MHK. It's very possible we'll need to figure out some way to convincingly lie to (and ignore commands from) the MHK to get certain things to work properly. We'll have to figure it out, and one of the hopes for making this an ESPHome project is that we'd be able to leverage the larger community to build the best possible controls we can.
Good news: we can! It's hidden over in M-NET and a pain to work with, but there's definitely a setting available (depending on your unit). Unfortunately, M-NET is out of scope for this project and likely will remain such. Might be worth picking up a diagnostic kit though... |
|
It's also worth noting that "Auto" for Mitsubishi systems is most equivalent to ( https://esphome.io/components/climate/#climate-control-action ) I'm not 100% sure, but I seem to remember that we've already implemented |
|
I've tested this PR on my mini-split and it seems to be working great, Any plans to expose vane/swing availability/commands in this PR? |
|
We do have those implemented, but were asked by the maintainers specifically to exclude those from this PR to speed up the review process. You can follow the directions here for using Glad to hear it's working well for you! |
|
Just going to do a quick, polite bump of this PR since there's been no progress. I've reduced the functionality of the component to make the PR smaller, is there anything else I can do to help move this forward? |
|
I'm successfully using this to control my single and multi split units, and it works great! thanks for the good work, and looking forward for this to be merged. |
|
Glad to hear it's working! There's probably at least a dozen or so people using it successfully now for quite some time, so functionally it seems fine. It's mostly just a matter of making sure the code is appropriate for merging at this point I think. |
|
My Mitsu A/C is finally on the way towards me so fingers crossed... |
|
@Sammy1Am the doc likely reflects the features of the previous PR, because trying to configure any of the sensors, buttons, and selects throws an error. The part I'm missing the most is the select to specify the temperature sensor to control the unit with... |
|
Ah, yeah, the new PR has been pared down at the request of ESPHome. I think you'd be best off using If you're looking for stability, you can just reference a particular commit instead of |
|
I think that in order for this PR to be reviewed/merged the corresponding documentation needs to be in par with it. So this either means to strip down the doc to the functionality implemented by this PR. Or add back the missing functionality here (but I guess that's not a good idea for now). I'll start testing importing the full component from your repo, thanx. |
(A little bit of a drive-by comment: I'm not the author but just an interested party/occasional contributor; I do not speak for the mUART folks). I understand the need to split into smaller components for ease of review, but isn't asking folks to also split up the documentation PRs a little bit of a tall order? It's quite a bit of extra work, that will effectively go to waste. Plus, that's not what happened with e.g. LVGL either. IIRC the code was merged over multiple PRs in stages, but the docs were all merged in one go. |
|
Yes but those stage PRs were also mostly ready and they were all merged into a single release. And LVGL was pretty popular and asked by many users so yes, it got quite some attention... For It's just that you can't have a doc describing non-existing functionality in a release... |
|
As inconvenient and messy as it will be, I think stripping the documentation is the right move in this case. It's been 5 months since the initial PR was opened, and 2 months since there's been any progress made on getting it reviewed, so I can't imagine multiple PRs getting done within a release cycle. For the same reason though, I'm going to hold off on the documentation until there's some kind of progress on the PR. The result of the last review was a big scope change, so I don't want to keep making big changes to the docs over and over. Edit:
In theory the features are all already implemented, but depending on how much more change results from the PR, they might need tweaking before they're ready for merging. |
That's why I think the doc need to be stripped down. It can't be skipped... |
|
Confirming now that this component works perfectly on a C3 Supermini attached to the unit's CN105 connector (I also used level shifters to be on the safe side). Changes in HA are instantly reflected on the wall panel, and changes on the panel reflect back in HA. SLZ-M35FA indoor cassette type, PAR-41MAA wall panel and SUZ-M35VA outdoor unit. |
| const uint8_t PACKET_HEADER_INDEX_PACKET_TYPE = 1; | ||
| const uint8_t PACKET_HEADER_INDEX_PAYLOAD_LENGTH = 4; | ||
|
|
||
| // TODO: Figure out something here so we don't have to static_cast<uint8_t> as much |
There was a problem hiding this comment.
Is static_cast<uint8_t> annoying to type explicitly or something else is a issue here?
There was a problem hiding this comment.
Unfortunately when you use enum class, typing and checking is stronger, with the downside you cannot automatically cast to the primitive.
There was a problem hiding this comment.
std::to_underlying, idk if its available in this c++ ver.
Best idea that I can think of is constexpr overator overloading.
For example:
constexpr uint8_t operator+(PacketType type) {
return static_cast<uint8_t>(type);
}
uint8_t var = +PacketType::CONNECT_REQUEST;
toValue is shortest to type compared to toUnderlying but still ugly.
constexpr uint8_t toUnderlying(PacketType type) {
return static_cast<uint8_t>(type);
}
uint8_t var = toUnderlying(PacketType::CONNECT_REQUEST);
Or just basic macro:
#define CAST_TO_U8(type) static_cast<uint8_t>(type)
Maybe create a class with bunch of static constexpr uint8_t defs. But its pointless
There was a problem hiding this comment.
As jesserockz says, the concern was less about having to type it out as it was about type safety. to_underlying seems like it would resolve this in most of the cases here, but it doesn't appear to be available in the limited embedded libraries.
Even though they wouldn't provide any additional safety, I do like your suggestions from a readability standpoint (might still incorporate something like toUnderlying next time I'm revising stuff).
| namespace mitsubishi_itp { | ||
|
|
||
| // Creates an empty packet | ||
| Packet::Packet() { |
There was a problem hiding this comment.
Just Packet(); or nothing (default constructor) in header would be enough if you don't plan to add something to consturctor in future
There was a problem hiding this comment.
Noted, will change with next update, thanks.
| const uint8_t packet_bytes[], uint8_t packet_length, SourceBridge source_bridge = SourceBridge::NONE, | ||
| ControllerAssociation controller_association = ControllerAssociation::MITP); // For reading or copying packets | ||
| // TODO: Can I hide this constructor except from optional? | ||
| RawPacket(); // For optional<RawPacket> construction |
There was a problem hiding this comment.
Make RawPacket(); private. Add friend class std::optional<RawPacket>; under private. That will make private members like RawPacket(); constructor accessible to std::optional. Just make sure that other private members are not accessible in other use cases with std::optional
There was a problem hiding this comment.
We dont use private in ESPHome
There was a problem hiding this comment.
I know, but it fulfills Can I hide this constructor except from optional?. As I understood that constructor is not used by users, only by std::optional. If private is that strictly prohibited then I don't see other solution.
Just make sure that other private members are not accessible in other use cases with
std::optional
Ahh I see, I don't know why I wrote that :)
There was a problem hiding this comment.
Still helpful information even if I can't use it in this case. Thank you.
|
Any progress on this? |
|
Oh my gosh, hallelujah!! Thank you so much!! I've been trying for years to get SwiCage/geoffdavis to work with my mini splits, but it never has. Plug in this component, and boom, works first time with no other changes! I should also note that I'm using ESP IDF framework, not Arduino, without issue. |
|
This has been working for me flawlessly since 6 months now. |
|
Same here! |
|
Thanks to those who’ve indicated success with this component. For someone who hasn’t gotten started with it yet, would it be better to wait for it to be merged, or use the development version? Forgive me, but I’m not too familiar with the ESPHome roadmap. |
It depends on your situation. If you already have the geoffdavis version working for you, I wouldn't spend time switching to this component until it's merged. If you don't even have an ESP device hooked up to your mini split yet, it's a judgement call if you should go ahead immediately, or wait until this is merged before trying things. If you're struggling with the geoffdavis component, or are simply ready to go ahead immediately with a new device, by all means, use the development version of this. In my experience it can take quite a long time for new components to merge into ESPHome. |
It's been over 10 months since the PR for this module was opened with no appreciable progress toward a merge, so if you're looking for a solution soon I wouldn't wait on the merge. We try to keep even the Where we're going to try to keep a separate If this PR does ever get picked up, there may be a bit more work to get it back up to date again, so that may also slow availability in ESPHome. |
|
Thanks to you both - appreciate your insight. I have an existing setup and was first running the geoffdavis version, and now have https://github.com/echavet/MitsubishiCN105ESPHome in place, but was looking forward to a solution fully integrated with ESPHome. @Sammy1Am, I will keep an eye out for the new |
What does this implement/fix?
Replacing #6794, this PR adds the
mitsubishi_itpcomponent that allows communication with Mitsubishi heat pumps via the CN105 connector. This integration works similarly to the seminal SwiCago/HeatPump library, except it has been written from the ground-up to be an ESPHome component. It also supports a second UART connection to an MHK2 thermostat for ease of control and integration with existing family workflows 😉.This PR contains only the
climatecomponent and core logic; sensors, buttons, and selects will be added in future PR(s).Types of changes
Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#3860
Test Environment
Example entry for
config.yaml:Additional configuration examples (including adding an MHK2 thermostat and using a Home Assistant sensor as a temperature source) are available here: https://muart-group.github.io/user/configuration
Checklist:
tests/folder).If user exposed functionality or configuration variables are added/changed: