SwiCago / HeatPump

Arduino library to control Mitsubishi Heat Pumps via connector cn105
GNU General Public License v3.0
828 stars 230 forks source link

Added support for getting and setting generic function codes #179

Closed akamali closed 2 years ago

akamali commented 2 years ago

This is based on monitoring communication between MHK1 and an air handler (PVA) and some Mitsubishi technical documents.

References:

In order to read the current value of function codes MHK1 sends the following packets: FC 42 01 30 10 20 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 5D FC 42 01 30 10 22 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 5B

The following is returned by the unit: FC 62 01 30 10 20 06 09 0D 11 16 18 3D 41 45 48 4C 50 54 58 00 8F FC 62 01 30 10 22 1F 22 24 29 2E 31 36 38 5D 61 65 69 6D 72 00 75

In this PR I'm incorporating this functionality. Since the function codes seem to depend on the models I'm keeping the implementation generic and it is up to the caller to find out what each function code controls.

I've also added support to connect to the heatpump using non-default serial pins, for people who use chips such as ESP32 that has multiple serial outputs.

SwiCago commented 2 years ago

@akamali can you remove the Header part of your packets and use the constants from .h instead Example, your get command starts with same bytes as INFOHEADER with added 20/22 Same for your set command, please use first 5 bytes of HEADER with 1F/21 or add a smaller HEADER for function with only the first 5 bytes. Look at how createInfoPacket works to prepend the header, then the data bytes, then the added checksum Your side for gets should only need to store something similar to like CONTROL_PACKET_1/2 in .h file. Best would be to have those as FUNCTION headers So you would use Shortened header, plus the function byte and auto fill the rest with 0x00 createPacket does. Follow similar syntax. So I would say, first auto gen the empty byte set, then replace the header bytes + function byte...send Same can be done with set. Note both set only support 14bytes, the last byte before checksum is 0x00, don't expect anyone to have to set that one. And do a check that all bytes are set. Also, I am not sure if your code pulls the codes first, if not make sure it has to pull the current codes before allowing to set them. I am sure you have it, but I did not go to deep into it.

If you can clean this up a little, I will gladly apply the pull. Looks promising so far. Maybe also add a textual note as to what it was tested on.

akamali commented 2 years ago

@SwiCago Thanks for the feedback. I addressed your comments about reusing the headers and I took the liberty to refactor some existing code instead of repeating the pattern. Suggested sanity checks about checking for 0 in byte 15 and making sure others are set is added as well, though the code is structured in a way that makes it difficult for those conditions to not be true, see my explanation below.

I am not sure if your code pulls the codes first, if not make sure it has to pull the current codes before allowing to set them.

The pattern I'm using requires passing an object to the set call. In order to get that object you need to call the get method:

  1. codes = getFunctions()
  2. codes.set(125, 2)
  3. codes.set(127, 1)
  4. setFunctions(codes)

As part of the set calls the code ensures that you can only modify a function that exists and it replaces that particular position in the buffer, all other values remain the same. Let me know if you have a better pattern in mind.

As for testing I have some local set up to use a new mqtt topic in order to set functions. This is what I'm seeing: { "set": { "101": 1, "102": 1, "128": 1} } The following is sent and received:

FC, 42, 01, 30, 10, 20, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 5D
FC, 62, 01, 30, 10, 20, 05, 09, 0D, 11, 16, 18, 3D, 41, 45, 48, 4C, 50, 54, 58, 00, 90
FC, 42, 01, 30, 10, 22, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 5B
FC, 62, 01, 30, 10, 22, 1F, 22, 24, 29, 2D, 31, 36, 38, 5D, 61, 65, 69, 6D, 72, 00, 76
FC, 41, 01, 30, 10, 1F, 05, 09, 0D, 11, 16, 18, 3D, 41, 45, 48, 4C, 50, 54, 58, 00, B2
FC, 61, 01, 30, 10, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 5E 
FC, 41, 01, 30, 10, 21, 1F, 22, 24, 29, 2D, 31, 36, 38, 5D, 61, 65, 69, 6D, 71, 00, 99
FC, 61, 01, 30, 10, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 5E 

Then { "set": { "101": 2, "102": 1, "128": 2} } produces:

FC, 42, 01, 30, 10, 20, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 5D
FC, 62, 01, 30, 10, 20, 05, 09, 0D, 11, 16, 18, 3D, 41, 45, 48, 4C, 50, 54, 58, 00, 90
FC, 42, 01, 30, 10, 22, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 5B
FC, 62, 01, 30, 10, 22, 1F, 22, 24, 29, 2D, 31, 36, 38, 5D, 61, 65, 69, 6D, 71, 00, 77
FC, 41, 01, 30, 10, 1F, 06, 09, 0D, 11, 16, 18, 3D, 41, 45, 48, 4C, 50, 54, 58, 00, B1
FC, 61, 01, 30, 10, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 5E 
FC, 41, 01, 30, 10, 21, 1F, 22, 24, 29, 2D, 31, 36, 38, 5D, 61, 65, 69, 6D, 72, 00, 98
FC, 61, 01, 30, 10, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 5E 

Then { "set": { "101": 3, "102": 1, "128": 3} } produces

FC, 42, 01, 30, 10, 20, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 5D
FC, 62, 01, 30, 10, 20, 06, 09, 0D, 11, 16, 18, 3D, 41, 45, 48, 4C, 50, 54, 58, 00, 8F
FC, 42, 01, 30, 10, 22, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 5B
FC, 62, 01, 30, 10, 22, 1F, 22, 24, 29, 2D, 31, 36, 38, 5D, 61, 65, 69, 6D, 72, 00, 76
FC, 41, 01, 30, 10, 1F, 07, 09, 0D, 11, 16, 18, 3D, 41, 45, 48, 4C, 50, 54, 58, 00, B0
FC, 61, 01, 30, 10, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 5E 
FC, 41, 01, 30, 10, 21, 1F, 22, 24, 29, 2D, 31, 36, 38, 5D, 61, 65, 69, 6D, 73, 00, 97
FC, 61, 01, 30, 10, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 5E 

I've done some testing myself, but I'd appreciate if you/someone else can also do some sanity checks to make sure at least nothing else is broken (I noticed my serial pin changes were causing compilation issues with ESP8266 😅 I didn't see those before since I was compiling for ESP32).

SwiCago commented 2 years ago

Looks good to me...merging. Thanks