Events with complex objects about ecovacs-deebot.js HOT 20 CLOSED

@dbochicchio

I have started to integrate the structured events from your pull request.
Maybe you can take a look at the commit.
I have changed a few things in detail.

from ecovacs-deebot.js.

@mrbungle64 I'd like to map some events (mopping, for example) and get some meaning for a couple of others (water level, stop reason, etc). I'm busy with life and other things, but I'll definitely take a look and release my own mqtt bridge, using your component, before end of this month ;)

from ecovacs-deebot.js.

@dbochicchio
I have changed the event name from "LifeSpanStats" to "LifeSpan" and the event is now triggered only after all components have already been handled:

const r = {};
if (this.bot.components["filter"]) {
    ...
    r["filter"] = this.bot.components["filter"];
}

...

if (this.bot.components["filter"] && this.bot.components["side_brush"] && this.bot.components["main_brush"]) {
    this.emit("LifeSpan", r);
}

I also implemented the "LifeSpan" event for non 950 type models.

from ecovacs-deebot.js.

@dbochicchio

I have started to document the combined events in the wiki:
https://github.com/mrbungle64/ecovacs-deebot.js/wiki/Events#combined-events

from ecovacs-deebot.js.

Hi @dbochicchio

Just a question: why are you pushing so many different events, instead of just one with an object inside?

I was also thinking about it this morning 😉
In the case of the clean log it makes total sense I think, but in many other cases it is more difficult to implement because the Ecovacs API sends most of the events at different times and it is also different for the different models.

I'm working on a MQTT bridge, and I've successfully modified the ecovacsMQTT_json.js to handle one event, instead of multiple (ie, in case of a clean log):

var lastCleanLogs = {
	timestamp: this.bot.cleanLog_lastTimestamp,
	squareMeters: this.bot.cleanLog_lastSquareMeters,
	lastTime: this.bot.cleanLog_lastTotalTimeString,
	imageUrl: this.bot.cleanLog_lastImageUrl
	// type
	// stopReason
};
this.emit("LastCleanLogs", JSON.stringify(lastCleanLogs));

I've not extensively looking at the library and I don't know if this sounds too crazy to you, but I could contribute with a pull request, if you want help.

You are welcome to submit the pull request if you like. I will take a look at it then.

PS: love your library, I was able to completely map every commands/events to my mqtt broker!

Thanks for your feedback 👍🏻

from ecovacs-deebot.js.

Thanks for your reply @mrbungle64!

I admit I've only looked at ecovacsMQTT_JSON.js, since I own a deebot 950. I think this could be used for maps, cachedmaps, clean logs, last clean, lifespan, position, waterinfo, errors, network info, clean stats.

I'll try to push something in the upcoming days. thanks again!

from ecovacs-deebot.js.

@dbochicchio

Thanks for your reply @mrbungle64!

I admit I've only looked at ecovacsMQTT_JSON.js, since I own a deebot 950. I think this could be used for maps, cachedmaps, clean logs, last clean, lifespan, position, waterinfo, errors, network info, clean stats.

I'll try to push something in the upcoming days. thanks again!

okay 👍🏻

Maybe you can start with "clean logs" and lifespan.
In the case of "clean stats" am going to make the change myself yet, as I just implemented this some hours ago 😉

But please try to make small commits, because I have to make the changes also for XMPP and MQTT/XML based models.

from ecovacs-deebot.js.

@dbochicchio

In the case of "clean stats" am going to make the change myself yet, as I just implemented this some hours ago

I just made a commit for "clean stats": c8164f5

from ecovacs-deebot.js.

@mrbungle64 great! I also did it for

• position
• lifespan
• error
• waterinfo
• networkinfo
• cleansum (what you've now renamed into Clean stats - OK for me)
• CleanLogs
• LastCleanLogs

See #73

from ecovacs-deebot.js.

@dbochicchio

Thanks, but please consider that the libary is used by some other projects (incl. 2 of my projects).
That's why I had suggested that you can start with "clean logs" and lifespan and also to make small commits, because I have to make the changes also for XMPP and MQTT/XML based models.

cleansum (what you've now renamed into Clean stats - OK for me)

I haven't renamed it. Can you please show me what you mean?

from ecovacs-deebot.js.

@mrbungle64 I'm sorry, I didn't notice that you just added it as a new option, and I was confused. Never mind.

Feel free to take it as inspiration, or simply ignore it. I'm running it with an MQTT bridge and I'm finding better to just intercept a generic status change, instead of multiple ones, with full status being sent instead of small increments. I think you'd just use new event names (as I did) while retaining the old names as well, or just use a flag. Up to you, but I'd help if you need it.

While we're at it, I had some troubles in understanding where to put logic and I would suggest to try to port it to a single point, and handle different payloads in a central place, instead of implementing it 3-4 times in different handlers.

All that said, keep up the good work! As I've said, I love your library!

from ecovacs-deebot.js.

@dbochicchio

Feel free to take it as inspiration, or simply ignore it. I'm running it with an MQTT bridge and I'm finding better to just intercept a generic status change, instead of multiple ones, with full status being sent instead of small increments. I think you'd just use new event names (as I did) while retaining the old names as well, or just use a flag. Up to you, but I'd help if you need it.

Yes, I will take it as inspiration because I also prefer to the proposed approach.
I also was thinking about retaining the old events for compatibility reasons.

This library originally was a fork of the JS port of the Python project sucks. I have added support for many models, which also differ in functionality and also often in detail. Not to mention that there's no official documentation of the API from Ecovacs 😅
That's why the events were added often one by one.

but I'd help if you need it.

I appreciate that. Thank you very much!

While we're at it, I had some troubles in understanding where to put logic and I would suggest to try to port it to a single point, and handle different payloads in a central place, instead of implementing it 3-4 times in different handlers.

Can you please give me some more information? Maybe we can convert this issue into a discussion.

from ecovacs-deebot.js.

@mrbungle64 I'm referring to handleCommand, that's defined in both base class (ecovacs.js) and specialized ones, like ecovacsMQTT_JSON.js: So, as you've stated, it's necessary to change the events in multiple parts. Maybe something into a central location, with optional info depending on the supported APIs, could lead to less work on you part to maintain the code.

from ecovacs-deebot.js.

@dbochicchio

@mrbungle64 I'm referring to handleCommand, that's defined in both base class (ecovacs.js) and specialized ones, like ecovacsMQTT_JSON.js: So, as you've stated, it's necessary to change the events in multiple parts. Maybe something into a central location, with optional info depending on the supported APIs, could lead to less work on you part to maintain the code.

I agree 👍🏻 I started to consolidate the handleCommand method in February and finished to consolidate it for XMPP and MQTT/XML based models but I haven't started doing it for MQTT/JSON too, because it's a bit more complicated.
But that should be one of my next goals 😅

from ecovacs-deebot.js.

@dbochicchio

There was something wrong with that pull request now. I had to make some corrections after merging it.
Maybe you can delete your repo and fork a new copy before continue working on it.

from ecovacs-deebot.js.

@mrbungle64 yep, I'm doing it while we speak... I've a couple of additions to push.

from ecovacs-deebot.js.

@dbochicchio
I merged your Pull Request. Thanks 👍🏻
Are your currently plaing to add some more additions?

from ecovacs-deebot.js.

@dbochicchio

I have just added an event (MapDataObject) for the map data: 2cd0d3b

from ecovacs-deebot.js.

@mrbungle64 great! I'm running a forked version combining water info into a single object, so I kwow in my broker if it's mopping or not.

// report+waterinfo
if (this.bot.cleanReport != undefined && this.bot.waterboxInfo != undefined)
	this.emit("CleanReportDetails", {
		'status': this.bot.cleanReport,
		'waterInfo': {
			'enabled': this.bot.waterboxInfo,
			'level': this.bot.waterLevel
		}
	});

I'll probably submit a PR in the next day or two.

from ecovacs-deebot.js.

@mrbungle64 great! I'm running a forked version combining water info into a single object, so I kwow in my broker if it's mopping or not.

// report+waterinfo
if (this.bot.cleanReport != undefined && this.bot.waterboxInfo != undefined)
	this.emit("CleanReportDetails", {
		'status': this.bot.cleanReport,
		'waterInfo': {
			'enabled': this.bot.waterboxInfo,
			'level': this.bot.waterLevel
		}
	});

I'll probably submit a PR in the next day or two.

from ecovacs-deebot.js.