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 converter for modern moes bht-006 thermostat #8005

Merged
merged 10 commits into from
Sep 24, 2024

Conversation

dukobpa3
Copy link
Contributor

Fixed mapping issues described here
Introduced new one converter for this device (mostly copypaste from previous)

Koenkk/zigbee2mqtt#24001

src/devices/moes.ts Outdated Show resolved Hide resolved
@dukobpa3 dukobpa3 requested a review from Koenkk September 18, 2024 19:26
@dukobpa3
Copy link
Contributor Author

Is it reasonable to remake it in modern style?

@Koenkk
Copy link
Owner

Koenkk commented Sep 19, 2024

That will require quite some changes, it's fine if you add just the expose definition to the existing BHT-002-GCLZB

@dukobpa3
Copy link
Contributor Author

That will require quite some changes, it's fine if you add just the expose definition to the existing BHT-002-GCLZB

there is different definitions. That's the issue.

– different controls for deadzone and max temp (45-70 vs 0-45) – in current defined 3 points. 2 of them wrong for this thermostat 3-d is not using at all.
– all controls should be integer not float
– different limits, steps and enum values in exposes.

I can try to make one definition but there will be spaghetti with a lot of if/elses

@dukobpa3
Copy link
Contributor Author

@Koenkk
Ok lets do this way

@@ -1134,6 +1134,14 @@ export const presets = {
)
.withValueMin(0)
.withValueMax(35),
max_temp: () =>
new Numeric('max_temp', access.STATE_SET)
Copy link
Owner

Choose a reason for hiding this comment

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

max_temperature_limit to be in sync with the min_temperature_limit below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

check lines 1127 and 1129

Copy link
Owner

Choose a reason for hiding this comment

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

Can you update moes_thermostat_max_temperature_limit instead? Use the correct datapoint

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is 18 but needed 19

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Different points for most devices and this one

                device?.manufacturerName === '_TZE204_aoclfnxz'
                    ? e.max_temp().withValueMin(45).withValueMax(70)
                    : e.max_temperature_limit().withValueMax(45),

Copy link
Owner

Choose a reason for hiding this comment

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

But you can do an if/else in moes_thermostat_max_temperature_limit:

if (device?.manufacturerName === '_TZE204_aoclfnxz') {
  // send to 19
} else {
  // send to 18
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Two layers of checking manufacturer name looks like layered spaghetty ))

But I'll look to it, ok

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok but then reasonable to do the same with deadzone

src/lib/legacy.ts Outdated Show resolved Hide resolved
@dukobpa3
Copy link
Contributor Author

So will you merge it? Or do you need some other changes?

@Koenkk
Copy link
Owner

Koenkk commented Sep 23, 2024

This comment is still left; #8005 (comment)

@Koenkk Koenkk merged commit ad72d4a into Koenkk:master Sep 24, 2024
2 checks passed
@dukobpa3 dukobpa3 deleted the feature/Moes_BHT-006-GB-ZB_converter branch September 26, 2024 19:39
dukobpa3 added a commit to dukobpa3/zigbee-herdsman-converters that referenced this pull request Oct 2, 2024
@Drafteed
Copy link
Contributor

Drafteed commented Oct 3, 2024

This should be reverted: #8063 (comment)

@Koenkk
Copy link
Owner

Koenkk commented Oct 4, 2024

Could you make a PR to revert it? Reverting through GH is not possible anymore because of conflicts.

@dukobpa3
Copy link
Contributor Author

dukobpa3 commented Oct 4, 2024

Ok I will

But what should I do with my device then?

@Drafteed
Copy link
Contributor

Drafteed commented Oct 5, 2024

Wonder if is it possible to check the fw version of Tuya devices in converters?

@dukobpa3
Copy link
Contributor Author

dukobpa3 commented Oct 5, 2024

I don't know but sure there definitely should be some differences. Moes/Tuya apps can recognise them somehow...

@Koenkk
Copy link
Owner

Koenkk commented Oct 5, 2024

Could you both post the data/database.db entries of your device?

@Drafteed
Copy link
Contributor

Drafteed commented Oct 5, 2024

@Koenkk

{
  "id": 25,
  "type": "Router",
  "ieeeAddr": "0xa4c138f863cdfe54",
  "nwkAddr": 16945,
  "manufId": 4417,
  "manufName": "_TZE204_aoclfnxz",
  "powerSource": "Mains (single phase)",
  "modelId": "TS0601",
  "epList": [
    1,
    242
  ],
  "endpoints": {
    "1": {
      "profId": 260,
      "epId": 1,
      "devId": 81,
      "inClusterList": [
        4,
        5,
        61184,
        0
      ],
      "outClusterList": [
        25,
        10
      ],
      "clusters": {
        "genBasic": {
          "attributes": {
            "65503": "\u001f�.i�\u001f�.i�\u001f�.i�\u001f�.i�\u001f�.i�\u001f�.i�\u001f�.i�\u001f�.i",
            "65506": 56,
            "65508": 0,
            "modelId": "TS0601",
            "manufacturerName": "_TZE204_aoclfnxz",
            "powerSource": 1,
            "zclVersion": 3,
            "appVersion": 74,
            "stackVersion": 0,
            "hwVersion": 1,
            "dateCode": ""
          }
        }
      },
      "binds": [],
      "configuredReportings": [],
      "meta": {}
    },
    "242": {
      "profId": 41440,
      "epId": 242,
      "devId": 97,
      "inClusterList": [],
      "outClusterList": [
        33
      ],
      "clusters": {},
      "binds": [],
      "configuredReportings": [],
      "meta": {}
    }
  },
  "appVersion": 74,
  "stackVersion": 0,
  "hwVersion": 1,
  "dateCode": "",
  "zclVersion": 3,
  "interviewCompleted": true,
  "meta": {},
  "lastSeen": 1728133559440
}

@dukobpa3
Copy link
Contributor Author

dukobpa3 commented Oct 5, 2024

{
  "id": 22,
  "type": "Router",
  "ieeeAddr": "0xa4c138baa855a68a",
  "nwkAddr": 42956,
  "manufId": 4417,
  "manufName": "_TZE204_aoclfnxz",
  "powerSource": "Mains (single phase)",
  "modelId": "TS0601",
  "epList": [
    1,
    242
  ],
  "endpoints": {
    "1": {
      "profId": 260,
      "epId": 1,
      "devId": 81,
      "inClusterList": [
        4,
        5,
        61184,
        0
      ],
      "outClusterList": [
        25,
        10
      ],
      "clusters": {
        "genBasic": {
          "attributes": {
            "65503": "$ܓ.g",
            "65506": 56,
            "65508": 0,
            "modelId": "TS0601",
            "manufacturerName": "_TZE204_aoclfnxz",
            "powerSource": 1,
            "zclVersion": 3,
            "appVersion": 74,
            "stackVersion": 0,
            "hwVersion": 1,
            "dateCode": ""
          }
        }
      },
      "binds": [],
      "configuredReportings": [],
      "meta": {}
    },
    "242": {
      "profId": 41440,
      "epId": 242,
      "devId": 97,
      "inClusterList": [],
      "outClusterList": [
        33
      ],
      "clusters": {},
      "binds": [],
      "configuredReportings": [],
      "meta": {}
    }
  },
  "appVersion": 74,
  "stackVersion": 0,
  "hwVersion": 1,
  "dateCode": "",
  "zclVersion": 3,
  "interviewCompleted": true,
  "meta": {},
  "lastSeen": 1728133642460
}

@Drafteed
Copy link
Contributor

Drafteed commented Oct 5, 2024

@Drafteed
Copy link
Contributor

Drafteed commented Oct 5, 2024

It looks like we need to look for differences in the data points, I think there is some value there indicating the revision of device.

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.

3 participants