search

alikh31 / node-red-contrib-eq3-bluetooth

node-red binding to bluetooth eq3 radiator valves without Max! Cube
MIT License
10 stars 5 forks source link

eq3device/eq3interface: #12

Open btsimonh opened 5 years ago

btsimonh commented 5 years ago

Hi Alikh31,

I've been working a lot with these devices now, and returned to noble (trying to make it work better!).

One of the things I've done is some heavy updates on the two lib files; in theory these mods don't change anything related to the node-red node, but I've NOT tested with the actual node. (all testing in function nodes so far). If you are able to test in your setup, that would be great.

Details: Make ALL sends parse the return. Enhance return parsing. add functions for schedules.
fix getInfo to setDate (it was screwing with my date/time on devices? - using the last time sent to it!). fix various '0' fill left issues. Add CC-RT-M-BLE. return {error:} on error rather than nothing. enhanced ecoMode ('holiday mode') setting with temp & date options.

I AM successfully using multiple devices with noble now. So my next task is to review the node functionality with regard to what I want to achieve, and see if I can enhance it without disturbing existing users :). If I can, I'll provide another pull request.....

btsimonh commented 5 years ago

just did a VERY quick test, and it the NR node seems to work :)

btsimonh commented 5 years ago

I notice in the node that you are setting a global[config.eq3device] = device. Do you use this externally (e.g. in a function node), or do you know if anyone else uses it?. One of the things I see as a need is to connect only when needed; else I don't think the 'phone app can work with the TRVs. Also connection management is quite evil in noble. I'm thinking about a similar approach of using a global, but to track all devices we've seen. Another thing I do is serialise all commands to each device. Also, some aspects of BLE are machine wide (e.g. scanning), and need to be dealt with appropriately. Other BLE related nodes will interfere without a little thought going in.

sorry, for the essay, but just thinking about how to approach the next step. Another thing I'd like to consider is if the EQ3 devices are access over a BLE to MQTT gateway (if I can successfully build one! - built but not yet blown to my ESP32....)

alikh31 commented 5 years ago

@btsimonh thanks for your contribution. unfortunately I have no eq3 device in hand at the moment. if you are sure the code will fully function I am more than happy to merge it. would be great to test it further and make sure everything is in place.

If there are any changes to the api please update the readme file as well.

btsimonh commented 5 years ago

ok, will do more testing. thankyou...

alikh31 commented 5 years ago

@btsimonh is this ready to merge?

btsimonh commented 5 years ago

hi @alikh31, give me a couple more weeks. It's pretty much there for what it does, but I'm still working hard on BLE in general, and that may result in some more re-work. s

btsimonh commented 5 years ago

@thomasnordquist - ready for re-review :). p.s. the NR node itself is ineffective for me at the moment because of the quantity of devices I have (10). But would be good to test the node itself with these changes, so that @alikh31 can merge with confidence.

my development flow is here: https://gist.github.com/btsimonh/178e1e736c7e4a8b0a549991c9331d7c note that I have 'require' in my settings file (live on the edge!). This flow is by no means final, but what it attempts to do is: Only connect to one device at a time (to get over BLE connection limits on platforms). Serialise all commands. You can ignore the mqtt aspects for now; but I will want to use the 'eq3interface' with BLE data over MQTT in the end. I'm on RPi3 with latest(?) bluez (pain in the a**e). I'm considering something like: https://github.com/shmuelzon/esp32-ble2mqtt/issues/6 but esp32 too unstable for the moment.

Off topic, but I can highly recommend this thermometer for ease of BLE integration, unless you are concerned about data security... (info is sent in advertisment, so simple, and no connection required).

thomasnordquist commented 5 years ago

I installed your branch and everything checks out so far. I'd like to extend the functionality of the actual node-red node as well, so expect a PR on your branch later today 😅.

thomasnordquist commented 5 years ago

By the way, nice thermometer. Even if it had no bluetooth 😅 I'll definitely order one.

btsimonh commented 5 years ago

p.s. my dev flow has a fatal (2 clicks and you are dead fatal) flaw. Referencing noble devices in global in NR 0.19 is BAD for your NR health. They added a 'Context' tab, and it loses ~30Mbytes per browser refresh. new one here ref in forum here

thomasnordquist commented 5 years ago

Can you narrow the problem down? Might be more helpful for the devs if they had something reproducible to toy with. Something like a minimal example without hardware requirements?

btsimonh commented 5 years ago

I tried, but the noble device is a VERY complex thing. could not reproduce with a simpler (very deep) structure. As I state in the forum, it could be just that the first time it actually fails, and breaks from then on. But it's gonna kill me when I update all my main stuff; I think the avoidance strategy is best; I do some evil stuff with contexts at the moment, all trying to avoid large messages (see 'clone'). To that forum message, I got one reply; I don't think others abuse NR as I do....!

thomasnordquist commented 5 years ago

Do you maybe have a cyclic reference? Some tree traversing algorithms don't check for cycles and therefore crash due to a stack overflow / limited memory. You could try to store something cyclic in the context and see if it crashes 🤨

var circularReference = {};
circularReference.circularReference = circularReference;

JSON.stringify(circularReference) // FAILS because it can detect cycles, and json doesn't support them
btsimonh commented 5 years ago

yep, tested with that :). browser shows 'Circular', and is fine. it's just one to avoid; there is not value having a large structure in the displayed context; it's great for 'normal' people :).

thomasnordquist commented 5 years ago

Do you have info on how to use https://github.com/btsimonh/node-red-contrib-eq3-bluetooth/blob/e27dfe72b1d3a1f851d8d05c96e3332f4466a522/lib/eq3interface.js#L36 ?

I'm documenting the node and how to use it, but the description is rather confusing: https://github.com/btsimonh/node-red-contrib-eq3-bluetooth/blob/e27dfe72b1d3a1f851d8d05c96e3332f4466a522/lib/eq3device.js#L120-L127

btsimonh commented 5 years ago

no, didn't play with it too much; so much guessing. The docs you highlighted are quite good though.....

thomasnordquist commented 5 years ago

Setting the temperature was no longer possible.

If you poll a lot, you might want to take a look at influxdb and grafana und 😉

alikh31 commented 5 years ago

Hi @btsimonh and @thomasnordquist sorry I've been away for a while. again since I have no physical devices I can not actually test the code but it looks legit to me. if you guys are happy with it I can merge the pr and release the package. btw since I am not highly available these days if you guys would like to be added as repo admin let me know so I add you guys.

janvda commented 5 years ago

Hi @btsimonh ,

I am currently testing branch btsimonh/node-red-contrib-eq3-bluetooth#btsimonh So it is working very well. Thanks a lot for that.

There are some minor glitches (like "list of all available addressess will be retrieved..." never returning a list) which I want to report as issue on your repository. But currently I don't have the privilege to report any issue on your repository. So it would be good if you could enable that feature for others.

kr Jan