Hypfer / Valetudo

Cloud replacement for vacuum robots enabling local-only operation
https://valetudo.cloud
Apache License 2.0
6.38k stars 388 forks source link

Extend map api to return the image directly #20

Closed rytilahti closed 5 years ago

rytilahti commented 6 years ago

First of all, thanks for working on this. From all the possible ways to access the map your approach seems to be the nicest one!

It would be nice to have an ability to request the generated image data directly without fetching the mapsrc-contained URL. A new parameter return_image=true could be added to return the PNG data (and proper HTTP content-type) as a response, which would allow direct integration to, e.g., homeassistant via https://www.home-assistant.io/components/camera.generic/

beloso commented 6 years ago

I am also interested in this feature.

beloso commented 6 years ago

I had an idea, you could save an alias/symbolic link for the latest generated image. And then serve it as you serve the other images VAC.IP/maps/latest.png

rytilahti commented 6 years ago

It would be much cleaner to simply have a parameter for the current API endpoint, assuming that the image has still to be separately generated, simply having a symlink for the generated image would not simplify the usage and can potentially cause confusion ("why is the latest map not updating?!"). That's why I see no reason for doing it as such (the API endpoint URL is already stable, and can be directly adapted to different usecases requiring different settings).

Bennedetto commented 6 years ago

If I understood you properly, this is already implemented in the Remote API. https://github.com/Hypfer/Valetudo#remote-api This creates a PNG onboard and returns the path of the just generated image. This should be easily usable on 3rd party software.

rytilahti commented 6 years ago

@Bennedetto yeah, I can see that it's returning a path to it, what I'd like to have is to allow returning the image data directly (as then there's no need on the 3rd party software to parse any JSON and handle it separately). So http://vacuum.local/api/remote/map?drawRobot=true&drawCharger=true&scale=5&border=3&doCropping=true&drawPath=true&returnImage=true would return a mime-type header (Content-Type: image/png) followed by the raw, generated image data. This could then be directly used to get the image (without parsing any JSON).

FelixUster commented 6 years ago

I've written some code to make this possible. In webserver/WebServer.js you need to do the following changes in the app.get("/api/remote/map", function(req,res) function:

(just search for the first line of every code block, that's the original code)

around line 400:

 var drawCharger = true;
 var drawRobot = true;
 var returnImage = true;

around line 420:

if (urlObj['query']['drawRobot'] !== undefined) {
      drawRobot = (urlObj['query']['drawRobot'] == 'true');
 }
 if (urlObj['query']['returnImage'] !== undefined) {
    returnImage = (urlObj['query']['returnImage'] == 'true');
}

around line 570:

var SS = (date.getSeconds()<10?'0':'') + date.getSeconds();
var fileName;
if (returnImage) {
   fileName = "latest.png";
} else {
   fileName = yyyy + "-" + mm + "-" + dd + "_" + HH + "-" + MM + "-" + SS + ".png";
}

around line 580:

function sendResult() {
    if (returnImage) {
        //res.sendfile(fileName, {root: tmpDir + directory});
        fs.readFile('/tmp/maps/latest.png', function (err, content) {
        if (err) {
           res.writeHead(400, {'Content-type':'text/html'})
           console.log(err);
           res.end("No such image");    
        } else {
           //specify the content type in the response will be an image
           res.writeHead(200,{'Content-type':'image/png'});
           res.end(content);
        }
   });
}  else {
       res.json({
             scale,
             border : border*scale,
             doCropping,
             drawPath,
             mapsrc : imagePath,
             drawCharger,
             charger : [homeX, homeY],
             drawRobot,
             robot : [robotPositionX, robotPositionY],
             robotAngle : Math.round(robotAngle)
      });
  }
}

now I can simply call: http://MIVACUUMIP/api/remote/map and get the latest map directly (other optional things should still work, I've only tested with the default parameters). I know that this code is far from good, but it is working. If someone can confirm that this code is ok, because it's the first time js coding for me, I would do a pull request

EDIT: added missing code in last code block

rytilahti commented 6 years ago

Thanks for working on this! Unfortunately I have no build setup around to test this out right now. One comment on the code though -- I'm not very familiar with javascript scoping, but instead of writing the image on the disk (or even if it's written as it is currently), but maybe you could simply use the existing image object inside the sendResult directly? This would avoid writing it first on the disk and then re-reading it back, and would avoid the changes around line 570. Just my 0.02e :-)

FelixUster commented 6 years ago

the renaming is really not necessary, but with this, I have a static link so I don't have to parse the send-back message and can easily pull the latest png without recreation.

rytilahti commented 6 years ago

Yup, I can see that. :+1: My proposal was to return the image data directly for the request, as that would allow one simply to specify the URL with wanted parameters and make just a single request to get what's wanted.

wills106 commented 6 years ago

@rytilahti If you are using Home Assistant have you seen my post about how I integrate Valetudo into my setup? https://community.home-assistant.io/t/valetudo-xiaomi-vacuum-dustcloud-alternative-live-maps/66277 I am wills106

rytilahti commented 6 years ago

@kgc210 I hadn't seen that, looks like a good solution for the time being (I'll try it out), the proposal here would allow simplify this by avoid the need for that rest sensor as you know, so I think this issue is still valid :-)

FelixUster commented 6 years ago

@rytilahti I don't know if I understand you correctly. If you want a single URL call (e.g. http://MIVACUUM/api/remote/map?drawCharger=true&returnImage=true ) to get the a freshly calculated image, that's what my code does. If you want the latest map WITHOUT recreation, this was not exactly the intention of my code, because I always want the latest image.

I've looked at my code in the code boxes again, and I was missing some lines, to still provide the original functionality

rytilahti commented 6 years ago

@FelixUster yes, your code does exactly what I want and I hope it gets merged in a form or another! I'm just question whether it's necessary to save that at all when it's being returned directly, even when the wear for doing this repeatedly every X seconds would not probably be that bad.

FelixUster commented 6 years ago

@rytilahti to you mean the wear of the flash storage? If you ssh into your robot and do df -h you can see that /tmp/, where all images are stored, is a tmpfs therefore in RAM, so I don't think it's critical. But yes, maybe someone with more knowledge could implement it in a better way =D

rytilahti commented 6 years ago

Ah, okay, I was mistaken! Why don't you create a PR and see what the @Hypfer thinks about it? :-) I currently have no development setup for ARM, so I cannot unfortunately can't give it a round now, but I hope we'll going to have such feature soonish in a valetudo release :-)

FelixUster commented 6 years ago

unfortunately my previous changes break the map view in den webgui, that's why I removed the part with the renaming to "latest.png"

I've created an PR, maybe we can see it in the next release :)

Hypfer commented 5 years ago

Replaced with mqtt map in 0.2.0