Replies: 20 comments 18 replies
-
Got this working nicely for kg only in https://github.com/DigiH/decoder/tree/DigiH-kg_only_test with additional mode (person or object) to allow for backend HA or openHAB to only record person relevant measurements. Example outputs are Object measurement Person measurement with shoes/socks on - no impedance reported Person measurement barefoot - impedance reported Trying to implement the same for all kg/lbs options in https://github.com/DigiH/decoder/tree/DigiH-XMTZC05HM-Extension resulted in some messy recognition of always object or lbs. Need to try and find out if this is due to some messed up formatting or something else tomorrow with a clearer head :) |
Beta Was this translation helpful? Give feedback.
-
Ok, would't let me rest as to why the complete decoder gave me such problems ;) This is the comparison of just adding the two lbs mode options to the correctly working kg only branch https://github.com/DigiH/decoder/compare/DigiH-kg_only_test...DigiH:DigiH-kg_plus_lbs?expand=1 I just add the additional two mode options and voila, the decoding doesn't work again constantly showing "mode":"object" now even with service data:02a4b2070101073307fdff0447 where
should definitely set it to person. What am I missing with the additional "__mode" and "___mode"? |
Beta Was this translation helpful? Give feedback.
-
Ok, I've created two new branches, along with @1technophile's hint to split the 2 and a variants so that the nested OR condition mentioned above isn't needed any longer. All working fine for kg only in https://github.com/DigiH/decoder/tree/kg_split_only (with corrected test cases now) but again, as soon as I add the appropriate lbs additions in https://github.com/DigiH/decoder/tree/kg_split_and_lbs , this time only to the a variant for the time being, the 2decoder is obviously still fine, but for the a the decoder always seems to pick the last entry for the mode, unit and weight decoding. Could it be that evaluating the same
twelve times for all the different options is tripping something over? ;) An other alternative trial could be to split up the decoder even more, adding the evaluation of "__unit":{ and
separately with another AND into the model condition, but this would create an unnecessary large amount of split decoders. Any idea what is blocking me here to implement the new features for XMTZC05HM for both kg and lbs? Thanks |
Beta Was this translation helpful? Give feedback.
-
Ok, I've done the latter idea, splitting the possible options in the model condition, only for the a variant for now. https://github.com/DigiH/decoder/blob/kg_lbs_a4_split/src/devices/XMTZC05HM_json.h and it is working fine now for a broadcasting kg and lbs decoding for all the different possibilities. If you can confirm that this is the only possibility for this feature implementation at the moment and that having 8 split decoders in the end is fine, I'll make the same changes to the 2 broadcasting option and append and amend the unit tests. Thanks |
Beta Was this translation helpful? Give feedback.
-
@h2zero do you think there are some possible decoder optimizations here? |
Beta Was this translation helpful? Give feedback.
-
@DigiH could we group these 2:
by leveraging a condition into the mode:
|
Beta Was this translation helpful? Give feedback.
-
Pulling back things together. But the unit and weight will then have to be doubled and conditioned as well
and then having the identical model condition for _XMTZC05HM_json_vkga and _XMTZC05HM_json_vlbsa, might that cause an issue?
or then just the kg combined and keeping the lbs split with the additional conditions ... Will try it out :) Thanks |
Beta Was this translation helpful? Give feedback.
-
So the pulling together both kg and lbs didn't work, likely due to the identical model condition
Splitting the lbs decoders again as with the previous 4 split still requires the combined kg to have the more universal above model condition, which works fine for recognising kg measurements, but the two more restrictive lbs model conditions
don't kick in and the more universal kg condition already catches and only publishes
Now trying to reorder the two more specific lbs decoders before the more universal kg decoder, to see if it makes any difference … |
Beta Was this translation helpful? Give feedback.
-
Yes, the reordering of the more specific two lbs decoders before the more universal kg decoder _XMTZC05HM_json_vlbsap _XMTZC05HM_json_vlbsao only then _XMTZC05HM_json_vkga works. There don't seem to be other decoders where the order is an issue/needs to be followed. My question now is, is it worth pulling _XMTZC05HM_json_vkgap and _XMTZC05HM_json_vkgao together, just to save one extra decoder (two when finally applied to the 2 variants as well), when now the evaluation order of the decoders in devices.h - const char _devices* has to be observed? https://github.com/DigiH/decoder/blob/kg_lbs_split_min/src/devices/XMTZC05HM_json.h or would the previous 4 (8) split not be more robust and unambiguous for now? |
Beta Was this translation helpful? Give feedback.
-
I'd like to walk this back a bit so it's objectively clearer. What would the ideal JSON decode input be? |
Beta Was this translation helpful? Give feedback.
-
So with the above all the 2 varaints should come back exactly like the currently working a variants with the awkwardly split 3 decoders. servicedata":"06 where it should be the same as save for the 2 (stabilised - scale still on) vs. a (stabilised - scale back in standby) difference, hence the initial nested model condition request. And the currently implemented three V2 tests should work:
with results
|
Beta Was this translation helpful? Give feedback.
-
@1technophile @h2zero I've started from scratch and cleanly implemented all the changes I think necessary, with currently two decoders for 2 and a stabilisation, including all possible 12 test cases. I hope this helps for clarification. https://github.com/DigiH/decoder/tree/XMTZC05HM-Extended Thanks |
Beta Was this translation helpful? Give feedback.
-
Thanks I was just looking into this, that helps :) |
Beta Was this translation helpful? Give feedback.
-
@DigiH I believe I have resolved this with a decoder modification. I have created a branch here: https://github.com/theengs/decoder/tree/ext-prop-conditions This also incorporates your JSON decode specs (removed the extra "va" type ) and fixes the tests to allow them to pass. Please test this with the actual device and let me know if it's suitable to your needs. |
Beta Was this translation helpful? Give feedback.
-
I've just done some quick testing before bedtime and things are looking great so far :) I'll do some more thorough testing tomorrow. Thanks a lot @h2zero! |
Beta Was this translation helpful? Give feedback.
-
@h2zero @1technophile Tested all 12 possible cases multiple times with my device and all is working perfectly as envisaged :) Thanks a lot for implementing this for the model and property conditions. I do have some questions about the implementation, hopefully without being too much of a pain. • Model condition
as the definition is not unambiguous without using any parenthesis, but the implementation here has been made to recognise it as
wouldn't it get confusing for someone who'd want to implement a model condition where something like this is needed
and very likely two separate decoders would be needed with the current implementation? I'm just thinking about future extended and more complex decoders and a clear definition for such condition AND OR nesting. Also with finding that the other Mi scale model XMTZC04HM also supports person/object distinction, but there this differentiation is on the same index as 'stabilised', it would need a model condition with currently four (kg only), but very likely 8, once lbs testing has been done. Would this case already be covered with the current implementation as well?
as in ( "b", "3", "f" and "7" currently only assumptions for the 'lbs' variations)
• Property conditions The current decoder with the mostly dual OR property conditions works really well, but thinking about resubmitting the iBeacon decoder to reduce its size by pulling together the power and battery conditions will the current implementation already allow for a longer OR chain?
It does look to me like it does briefly glancing at the changed code, but I'd need a lot longer to fully understand it all ;) Thanks |
Beta Was this translation helpful? Give feedback.
-
Yes, nesting conditions will likely be needed at some point in the future but it was not my intention for today :) It would require a fairly large change to the code and I'd prefer to wait until there is a case where it's required to facilitate better testing.
It should work the same.
It will keep looping on however many conditions you can fit :) |
Beta Was this translation helpful? Give feedback.
-
@DigiH If you would like to rebase your code with that branch and PR it with any further modifications please feel free to do so. Might as well keep it all in 1 PR. |
Beta Was this translation helpful? Give feedback.
-
@h2zero thanks, yes, I've already made a few adjustments and will do that. |
Beta Was this translation helpful? Give feedback.
-
Not quite what I expected, diggin into the tests ;) |
Beta Was this translation helpful? Give feedback.
-
@1technophile @h2zero With my comments in
1technophile/OpenMQTTGateway#760
and
1technophile/OpenMQTTGateway#1166
and trying to implement all these findings in the XMTZC05HM decoder I have some queries.
Is there a possibility to have nested & and | conditions to achieve something like
"condition":["uuid", "contain", "181b", "&",("servicedata", "index", 2, "2", "|", "servicedata", "index", 2, "a")],
if both those stabilised (load still on or off) conditions want to be accepted?
"servicedata", "index", 2, "2" only gets caught when still standing on the scale and the display is still on, the advantage being that once the scale display switches off the service data is not recognised any longer during BLE scans, but if the TimeBtwRead is too long it might not get recognised at all, then only the "servicedata", "index", 2, "a" is being recognised and also published during any consecutive scan process - which it already is with just "condition":["uuid", "contain", "181b"]
Any ideas on these issues?
Thanks
Beta Was this translation helpful? Give feedback.
All reactions