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

Model Condition Restrictions #107

Merged
merged 1 commit into from
Mar 30, 2022

Conversation

DigiH
Copy link
Member

@DigiH DigiH commented Mar 30, 2022

Model condition restrictions from "contain" to more specific "index", @

Checklist:

  • The pull request is done against the latest development branch
  • Only one feature/fix was added per PR and the code change compiles without warnings
  • I accept the DCO.

I haven't checked off the checklist yet as there are two issues I'd like to discuss first.

• For the VEGTRUG decoder there are no test cases, but as it seems to be a rebranded Xiaomi Mi Flora, similar to other plant devices which seem to be Xiaomi/Vegtrug rebranded/identical, I'm wondering if we shouldn't combine the two, apart from the model definitions, identical decoders and indicate this in the brand and model naming? MERGED the two decoders.
• The MUE4094RT Xiaomi Mi Lamp decoder seems to have property condition issues. All I could find was an OMG original implementation

1technophile/OpenMQTTGateway@bb1df8f

where the sample servicedata confirms the model condition string at index 0, but the currently existing property conditions with "4812" are nowhere to be found in this example and conflict with index 0.

@1technophile do you have any more info on the MUE4094RT so we can possibly amend the decoder and also include some test case?

Thanks

@DigiH DigiH force-pushed the modelcondition-restrictions branch 3 times, most recently from f617f5c to 2a9a2ea Compare March 30, 2022 15:15
@DigiH
Copy link
Member Author

DigiH commented Mar 30, 2022

@emericg as there seems to be a similar situation with your RoPot submission, that there is/was an identical VegTrug pot - do you know if and how the VegTrug differs with the model condition recognition so that we might include it as well?

@emericg
Copy link
Contributor

emericg commented Mar 30, 2022

@emericg as there seems to be a similar situation with your RoPot submission, that there is/was an identical VegTrug pot - do you know if and how the VegTrug differs with the model condition recognition so that we might include it as well?

I think what you call a VegTrug is the flower care max / grow care garden, not the "VegTrug" branded flower care (also call grow care home).
So no there shouldn't be any VegTrug RoPot.

@DigiH
Copy link
Member Author

DigiH commented Mar 30, 2022

So no there shouldn't be any VegTrug RoPot.

This is what I meant

https://www.lcgad.com/vegtrug-grow-care-smart-pot/

and looking at your

https://github.com/emericg/WatchFlower#supported-devices

where the RoPot states it has "Xiaomi and VegTrug variants", unless both variants are already covered with the current

"condition":["servicedata", "index", 2, "205d01"],

then we could just change the "brand":"Xiaomi" to "brand":"Xiaomi/VegTrug", similarly to my Mi Flora/VegTrug proposal.

@emericg
Copy link
Contributor

emericg commented Mar 30, 2022

Yes I know about the RoPot sold by VegTrug, but what I'm saying is that it's the exact same as the Xiaomi one. They are both manufactured by HHCC and sold under different brands.
The Flower Care from VegTrug are the same as the one from Xiaomi (and are also sold under other brands too, like Wanfei and even HHCC I think).
They all use the same 0x9800 product ID AFAIK. I've had some VegTrug but I have donated them (I have only the boxes left :).

The Flower Care Max / VegTrug Grow Care Garden however (presumably the one with 0xBC03 pid), seems to have a twist in its protocol (pending investigation, I don't have one) because I already have two reports of it not working exactly as intended.

@DigiH
Copy link
Member Author

DigiH commented Mar 30, 2022

The Flower Care from VegTrug are the same as the one from Xiaomi (and are also sold under other brands too, like Wanfei and even HHCC I think).

Yes, that was my question, while the property conditions and definitions are very likely the same, does the current model condition

"condition":["servicedata", "index", 2, "205d01"],

apply to both the Xiaomi version as well as the VegTrug version, as with the 'Mi Flora' there were differences in the model condition, while everything else was identical.

Just submitted the Xiaomi/VegTrug Mi Flora merge. With you not having the VegTrug Pot any longer I think we should leave that decoder as is and if it also recognises the VegTrug variant I'm sure people don't mind the 'Xiaomi' brand as long as they get the correct moisture and fertility information :)

@DigiH DigiH force-pushed the modelcondition-restrictions branch from 2a9a2ea to f6cf581 Compare March 30, 2022 16:50
@emericg
Copy link
Contributor

emericg commented Mar 30, 2022

I've never had a VegTrug pot, I never even hear of anyone else having a RoPot (until @1technophile told me he had one last week).
But look what I found searching for infos: https://fccid.io/2AQPCVGT-GC04-CAZ/User-Manual/user-manual-4461104
It seems to be a leg-less Flower Care. Another weird variant (and probably never seen in the wild :)

@DigiH
Copy link
Member Author

DigiH commented Mar 30, 2022

And all probably discontinued, so best to leave everything as is with the pots :)

Still struggling here with trying to find more actual servicedata for the MUE4094RT Mi Lamp to finish this clean up.

@DigiH
Copy link
Member Author

DigiH commented Mar 30, 2022

So reading more about the MUE4094RT Mi Lamp, and that it only seems to broadcast any data when it's dark and if there has been motion/presence detected, the property conditions should likely be deleted.

@DigiH DigiH force-pushed the modelcondition-restrictions branch from 3c2152e to 5de9b6a Compare March 30, 2022 18:48
@DigiH DigiH force-pushed the modelcondition-restrictions branch from 5de9b6a to 2004704 Compare March 30, 2022 18:52
@DigiH
Copy link
Member Author

DigiH commented Mar 30, 2022

Checklist:

  • The pull request is done against the latest development branch
  • Only one feature/fix was added per PR and the code change compiles without warnings
  • I accept the DCO.

Was it correct to delete the battery property for the Mi Flora HHCCJCY01HHCC, or does this need to stay in, even if it's not obtained by the broadcast data, but through the direct connection handling?

@DigiH DigiH requested a review from 1technophile March 30, 2022 19:45
@1technophile
Copy link
Member

Was it correct to delete the battery property for the Mi Flora HHCCJCY01HHCC, or does this need to stay in, even if it's not obtained by the broadcast data, but through the direct connection handling?

That's a good question.
The issue is that it will remove the battery auto-discovery done by OMG for the Mi Flora, the battery values are retrieved by an OMG BLE connect.
The MiFlora is particular because most of the values are retrieved real-time but the battery is retrieved through a connection.
So let's remove it and we can update OMG code to have a dedicated discovery for the MiFlora/Vegtrug battery.

@1technophile
Copy link
Member

Thanks

@1technophile 1technophile merged commit 971ebb3 into theengs:development Mar 30, 2022
@DigiH DigiH deleted the modelcondition-restrictions branch March 31, 2022 00:58
1technophile added a commit to 1technophile/OpenMQTTGateway that referenced this pull request May 3, 2022
1technophile added a commit to 1technophile/OpenMQTTGateway that referenced this pull request May 4, 2022
…tity id (#1209)

* Recovery MiFlora discovery

after theengs/decoder#107

* BREAKING remove model_id from entity id
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.

None yet

3 participants