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

Merge WPU manual control from fork #234

Merged
merged 4 commits into from
Feb 25, 2024

Conversation

tomkooij
Copy link
Contributor

@tomkooij tomkooij commented Feb 24, 2024

This merges the changes in the private fork that enables "manual operation". It is mainly used for controlling the WPU heatpump.

This commit adds "4030 Manual control" to the debug page. The debug option "dryrun" is removed. It is no longer needed for initial debugging.

Ref: https://gathering.tweakers.net/forum/list_message/77786836#77786836

It seems to work, but I will do some more testing tomorrow, to make sure this actually works.

TODO:

  • Test.
  • Add documentation to wiki

This merges the changes in the private fork that enables "manual
operation". It is mainly used for controlling the WPU heatpump.

This commit adds "4030 Manual control" to the debug page.
The debug option "dryrun" is removed. It is no longer needed.
@tomkooij tomkooij mentioned this pull request Feb 24, 2024
@tomkooij tomkooij changed the title Merge WPU manual operation from fork Merge WPU manual control from fork Feb 24, 2024
@arjenhiemstra
Copy link
Owner

Thanks!

I've pushed some textual changes and a first try for some documentation for the api page of the add-on.
Could you have a look and add some details to the key descriptions (I do not know the possible values and their function)
the basic text is already there.

https://github.com/arjenhiemstra/ithowifi/tree/pr/234

@arjenhiemstra
Copy link
Owner

and here also a compiled firmware file for those who want to test:
nrgitho-v2.7.4-pr234-test.bin.zip

@tomkooij
Copy link
Contributor Author

tomkooij commented Feb 25, 2024

I've tested this and it works OK.

I do notice something that might be better to fix. If I leave an extra comma in the MQTT command:

{
            "manual_operation_index":34,
            "manual_operation_datatype":0,
            "manual_operation_value":0,
            "manual_operation_checked":1,
}

Note the extra comma after "checked":1,
This is valid JSON, but the command is rejected (picked up as another type of command). Debug log shows:

Feb 25 07:39:52 nrg-itho-c2a8 nrgitho EXEC COMMAND:{ "manual_operation_index":30, "manual_operation_datatype":0, "manual_operation_value":0, "manual_operation_checked":1,#012}

instead of the D_LOG() echos of the command that is executed:

Feb 25 07:40:09 nrg-itho-c2a8 nrgitho index: 30 dt: 0 value: 0 checked: 1
Feb 25 07:40:09 nrg-itho-c2a8 nrgitho Sending 4030: 82 80 40 30 06 07 01 00 1E 00 00 00 01 61

@tomkooij
Copy link
Contributor Author

tomkooij commented Feb 25, 2024

I merged pr/234 into this and added (a little bit) of information. The information is from the (publicly available) documentation of the i2c modbus module: https://www.ithodaalderop.nl/nl-NL/consument/product/03-00638
(Table 8.4). I'm not sure if we should provide a link / reference to this source.

In the past I've added some documentation for these WPU features to the wiki: https://github.com/arjenhiemstra/ithowifi/wiki/Itho-WPU-Heat-pump-support
I will add something about manual control as well

EDIT: Done.

@arjenhiemstra
Copy link
Owner

arjenhiemstra commented Feb 25, 2024

I've tested this and it works OK.

I do notice something that might be better to fix. If I leave an extra comma in the MQTT command:

{
            "manual_operation_index":34,
            "manual_operation_datatype":0,
            "manual_operation_value":0,
            "manual_operation_checked":1,
}

Note the extra comma after "checked":1, This is valid JSON, but the command is rejected (picked up as another type of command). Debug log shows:

Feb 25 07:39:52 nrg-itho-c2a8 nrgitho EXEC COMMAND:{ "manual_operation_index":30, "manual_operation_datatype":0, "manual_operation_value":0, "manual_operation_checked":1,#012}

instead of the D_LOG() echos of the command that is executed:

Feb 25 07:40:09 nrg-itho-c2a8 nrgitho index: 30 dt: 0 value: 0 checked: 1
Feb 25 07:40:09 nrg-itho-c2a8 nrgitho Sending 4030: 82 80 40 30 06 07 01 00 1E 00 00 00 01 61

Had a small look into this. A JSON with a stray comma (one not followed by an object key or array value) is not a valid JSON. That is exactly what you see happening.

The input fails here:

DeserializationError error = deserializeJson(root, s_payload);

and hits this line:

as there is an error in the JSON, the API processing passed the input to the ithoExecCommand function here:

else
{
ithoExecCommand(s_payload, MQTTAPI);

what is passed is the raw input string (s_payload), which is logged in the ithoExecCommand function.

@arjenhiemstra
Copy link
Owner

I merged pr/234 into this and added (a little bit) of information. The information is from the (publicly available) documentation of the i2c modbus module: https://www.ithodaalderop.nl/nl-NL/consument/product/03-00638 (Table 8.4). I'm not sure if we should provide a link / reference to this source.

In the past I've added some documentation for these WPU features to the wiki: https://github.com/arjenhiemstra/ithowifi/wiki/Itho-WPU-Heat-pump-support I will add something about manual control as well

EDIT: Done.

nice work, thanks!!!

@arjenhiemstra
Copy link
Owner

I merged pr/234 into this and added (a little bit) of information. The information is from the (publicly available) documentation of the i2c modbus module: https://www.ithodaalderop.nl/nl-NL/consument/product/03-00638 (Table 8.4). I'm not sure if we should provide a link / reference to this source.

I just had a look at this link, imho it's best to leave as little external dependancies as possible.
but I read this: "Artikelprijs: € 449,00"
Whut?! Is that the price for something as simple as a i2c to modbus converter?!?! That's outrageously crazy

@arjenhiemstra arjenhiemstra merged commit d64a4f0 into arjenhiemstra:master Feb 25, 2024
1 check passed
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.

2 participants