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

add thermalcontrol module #40

Open
wants to merge 19 commits into
base: feature/59016-integrations-package
Choose a base branch
from

Conversation

schtangel
Copy link

Module for heating and cooling.
Now just deadband control.

schtangel added 2 commits May 19, 2023 14:09
Module for heating and cooling.
Now just deadband control.
@aadegtyarev
Copy link

aadegtyarev commented May 19, 2023

Для пользователя выглядит так:
изображение

Для интегратора/программиста так:
изображение

@schtangel schtangel marked this pull request as ready for review May 19, 2023 11:25
Changed titles and off status
modules/thermalcontrol.js Outdated Show resolved Hide resolved
modules/thermalcontrol.js Outdated Show resolved Hide resolved
modules/thermalcontrol.js Outdated Show resolved Hide resolved
modules/thermalcontrol.js Outdated Show resolved Hide resolved
modules/thermalcontrol.js Outdated Show resolved Hide resolved
* Changed order of controls
* Modified logging (format and logging levels)
* Changed config.modes from obj to array of strings
debian/changelog Outdated Show resolved Hide resolved
schtangel and others added 4 commits May 23, 2023 12:21
Add thermalcontrol module

Co-authored-by: Nikolay Korotkiy <[email protected]>
* added resistance to the absence of optional fields in the config
* added checks for the required fields
Add Hysteresis field to virtual device
Add persistent storage for setpoint
Copy link

@aadegtyarev aadegtyarev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не работает. Инициализирую так по примеру, только поправил скобку в строке 102:

var libtherm = require("thermalcontrol");

config = ({
    devName: "climate_kitchen",
    devTitle: "Температура на кухне",
    modes: ["heating", "cooling"],
    temperatureSource: "wb-w1/somesensor",
    heatingChannel: "wb-gpio/EXT2_R3A1",
    coolingChannel: "wb-gpio/EXT2_R3A2",
    setpointrange: {
        "min": 6,
        "max": 35
    },
    hysteresis: 1,
    degug: true
});

var climate_in_kitchen = new libtherm.ThermalControlDevice(config);

Получаю ошибку в модуле:

Ошибка правила: TypeError: invalid base value
duk_hobject_props.c:2000
anon /usr/share/wb-rules-system/scripts/lib.js:813 preventsyield
anon thermalcontrol:312 construct preventsyield
F /etc/wb-rules/thermostat-test.js:18 preventsyield

Ещё надо статус OFF заменить на DISABLED — это ближе к выключателю ENABLED по смыслу и не будет потом путаницы с OFF_RELAY.

modules/thermalcontrol.js Outdated Show resolved Hide resolved
schtangel added 4 commits May 25, 2023 10:20
Changed 'OFF' status to 'DISABLED'
not everywhere changed "OFF" to "DISABLED", now everywhere
* add default setpoint setting to config
* remove persistent storage
Copy link

@aadegtyarev aadegtyarev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Посмотрел — все ок. Оставил несколько комментариев, надо поправить и будет совсем хорошо.

Надо как можно больше параметров сделать необязательными с описанными в документации значениями: https://wirenboard.com/wiki/Wb-rules-modules#%D0%A1%D0%BE%D0%BA%D1%80%D0%B0%D1%89%D1%91%D0%BD%D0%BD%D0%B0%D1%8F_%D0%B7%D0%B0%D0%BF%D0%B8%D1%81%D1%8C

Давай поправим и вольём, пока я не ушёл в отпуск.

modules/thermalcontrol.js Show resolved Hide resolved
modules/thermalcontrol.js Outdated Show resolved Hide resolved
modules/thermalcontrol.js Show resolved Hide resolved
modules/thermalcontrol.js Show resolved Hide resolved
modules/thermalcontrol.js Show resolved Hide resolved
modules/thermalcontrol.js Outdated Show resolved Hide resolved
schtangel added 2 commits May 26, 2023 13:52
* Add units
* Add checks for setpoint
* Fixed cofusion with status
* Fixed JSDOC example
* Add check of topics exist
@webconn webconn requested review from a team and KraPete May 29, 2023 08:24
Copy link
Contributor

@KraPete KraPete left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Надо описание структуры топиков создаваемого в MQTT устройства. Надо описать возможные значения этих топиков, всякие SUSPEND, DISABLED и т.п.
  2. Может вместо SUSPEND писать IDLE? Встречал такое в холодильных установках
  3. Кто должен сохранять значения setpoint, установленные пользователем? Сейчас при любом перезапуске, они будут сброшены, и будет установлено значение по умолчанию. Это может быть неожиданно.
  4. Я бы всякие однообразные проверки наличия топиков вынес в отдельные функции, чтоб меньше повторений кода было.
  5. Надо ли обрабатывать ситуации, когда исчезли устройства, выдающие температуру и реализующие управление?
  6. Надо ли обрабатывать ошибку чтения у устройства, которое выдаёт температуру? Кажется, что стоит отключить нагрев и охлаждение.
  7. Для обсуждения: возможно, при создании термостата не надо выкидывать исключение, если нужных устройств нет в MQTT. Возможно стоит сигнализировать в топиках ошибкой, а при появлении устройств начинать работать, согласно настройкам?
  8. Для обсуждения: было бы удобно, если бы конфиг термостатов читался из файлика с диска. Такой конфиг можно было бы делать средствами homeui. Тогда пользователь мог бы настроить его сам без погружения в правила.

modules/thermalcontrol.js Outdated Show resolved Hide resolved
modules/thermalcontrol.js Outdated Show resolved Hide resolved
modules/thermalcontrol.js Outdated Show resolved Hide resolved
modules/thermalcontrol.js Outdated Show resolved Hide resolved
modules/thermalcontrol.js Outdated Show resolved Hide resolved
modules/thermalcontrol.js Outdated Show resolved Hide resolved
modules/thermalcontrol.js Outdated Show resolved Hide resolved
},
setpoint: {
title: "Setpoint",
type: "value",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Может, range сделать? В интерфейсе будет красивее выглядеть

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Да мы что-то так и эдак крутили это. А есть способ сделать шаг не в целый градус, а задаваемым?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

есть meta/precision, не уверен, что wb-rules это умеют

Copy link
Author

@schtangel schtangel Jun 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не умеют :(
Оставил пока value, всё-таки его по одному градусу двигать удобней.

modules/thermalcontrol.js Outdated Show resolved Hide resolved
modules/thermalcontrol.js Outdated Show resolved Hide resolved
schtangel and others added 4 commits May 30, 2023 10:21
* add jsdoc for virtualdevice
* minor fixes
* remove bindcontrols
* fix checks for heatingChannel/coolingChannel exist
* changed order
@schtangel
Copy link
Author

  1. Надо описание структуры топиков создаваемого в MQTT устройства. Надо описать возможные значения этих топиков, всякие SUSPEND, DISABLED и т.п.
  2. Может вместо SUSPEND писать IDLE? Встречал такое в холодильных установках
  3. Кто должен сохранять значения setpoint, установленные пользователем? Сейчас при любом перезапуске, они будут сброшены, и будет установлено значение по умолчанию. Это может быть неожиданно.
  4. Я бы всякие однообразные проверки наличия топиков вынес в отдельные функции, чтоб меньше повторений кода было.
  5. Надо ли обрабатывать ситуации, когда исчезли устройства, выдающие температуру и реализующие управление?
  6. Надо ли обрабатывать ошибку чтения у устройства, которое выдаёт температуру? Кажется, что стоит отключить нагрев и охлаждение.
  7. Для обсуждения: возможно, при создании термостата не надо выкидывать исключение, если нужных устройств нет в MQTT. Возможно стоит сигнализировать в топиках ошибкой, а при появлении устройств начинать работать, согласно настройкам?
  8. Для обсуждения: было бы удобно, если бы конфиг термостатов читался из файлика с диска. Такой конфиг можно было бы делать средствами homeui. Тогда пользователь мог бы настроить его сам без погружения в правила.

1 - добавил описание
2 - да, наверное так лучше. Тоже видел на регуляторах всяких.
3 - мне казалось, это силами самого wb-rules как-то делается? в любом случае, оно сейчас успешно переживает перезагрузку контроллера.
4 - да, надо. Посмотрю, какие из них можно нормально завернуть. С дебагом аналогично.
5 и 6 - стоит, сделаю вот прямо сейчас.
7 и 8 - надо будет обсудить с Сашей, да

add description of config.setpoint object
@aadegtyarev
Copy link

Дальше делает @aadegtyarev

@aadegtyarev
Copy link

Решили не делать. Вместо этого будет визуальный конфигуратор.

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.

4 participants