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

New homepage #248

Closed
wants to merge 10 commits into from
Closed

New homepage #248

wants to merge 10 commits into from

Conversation

edautz
Copy link
Contributor

@edautz edautz commented Jun 30, 2018

As promised!

edautz and others added 5 commits June 30, 2018 12:44
	docroot/static/js/datetimepicker_css.js
	docroot/static/js/images2/
	new file:   docroot/static/js/images2/cal.gif
	new file:   docroot/static/js/images2/cal_close.gif
	new file:   docroot/static/js/images2/cal_fastforward.gif
	new file:   docroot/static/js/images2/cal_fastreverse.gif
	new file:   docroot/static/js/images2/cal_forward.gif
	new file:   docroot/static/js/images2/cal_minus.gif
	new file:   docroot/static/js/images2/cal_plus.gif
	new file:   docroot/static/js/images2/cal_reverse.gif
@jpmens
Copy link
Member

jpmens commented Jul 5, 2018

Thank you for the submission which I'm looking at now.

As can be seen from this screenshot, I don't get a list of users shown, neither with Safari, Firefox, nor with Chrome. Neither browser shows an error message in the console.

What do you think this is due to?

jmbp-4539

@jpmens
Copy link
Member

jpmens commented Jul 5, 2018

In 1d90c4b you have created index.html which I think is unintentional; it ought to be docroot/index.html (at least the diff looks as though it should be that file).

@jpmens
Copy link
Member

jpmens commented Jul 5, 2018

Upon opening /, I see the Recorder get a request for api/0/last, and it actually is returning the user array.

Jul  5 18:23:25  ot-recorder[30180] <Debug>: http: GET /
Jul  5 18:23:25  ot-recorder[30180] <Debug>: http: GET /static/js/datetimepicker_css.js
Jul  5 18:23:25  ot-recorder[30180] <Debug>: http: GET /api/0/last
Jul  5 18:23:25  ot-recorder[30180] <Debug>: http: GET /static/js/images2/cal.gif
[
 {
  "_type": "location",
  "t": "L",
  "tid": "J4",
  "tst": 1471511155,
...

@edautz
Copy link
Contributor Author

edautz commented Jul 5, 2018

The index.html file from 1d90c4b is the last and the best one. The former ones contains several little bugs.

Strange that your userlist is not created. On my Win7 machine it runs fine also on my apple devices. It is not tested on win10.

Lookes like the following part of the code is not executed when loading the page or this part of the code is not gaining any data.

The first part of the code gets the output of /api/0/last, and creates two arrays:
var user =[];
var device =[];
and fills them with the extracted users and devices from /api/0/last output.

After this step, a loop creates the corresponding radio buttons for the users and devices.

Please put a debugger; statement in the code, and check if user[] and device[] array's are filled with your users and devices.

You can also send me your /api/0/last file, so I can test this file and the generated output on my machine.

+      <script>
+        //fetch user/device from url 
+        document.getElementById('error3').innerHTML = "";
+        //var dataURI = "http://192.168.2.113:8083/api/0/last";
+        var dataURI = "/api/0/last";
+        var Httpreq = new XMLHttpRequest(); // a new request
+        Httpreq.open("GET",dataURI,false);
+        Httpreq.send(null);
+        var urldata = Httpreq.responseText;          
+        if (urldata.length < 3) document.getElementById('error3').innerHTML = "OT-recorder user data couldn't be loaded";
+        //parsing users en devices
+        var stringpointer = 0;
+        var beginposdev;
+        var endposdev;
+        var user =[];
+        var device =[];
+        var index = 0;
+        // get users and devices
+        while (stringpointer < urldata.length) {
+          beginposus = urldata.indexOf("user", stringpointer);
+          beginposdev = urldata.indexOf("device", beginposus);
+          endposdev = urldata.indexOf("fulldate", beginposdev);
+          if (stringpointer < endposdev) stringpointer = endposdev;
+          else { break; }   
+          user[index] = urldata.substr((beginposus+11),(beginposdev-beginposus-14));
+          device[index] = urldata.substr((beginposdev+9),(endposdev-beginposdev-12));
+          index ++;
+        }
+
+
+        //build user/device radio button list
+        var radio_home = document.getElementById("usertable");
+        function makeRadioButton(name, useriddeviceid, useriddeviceidinput) {
+          var label = document.createElement("label");
+          var radio = document.createElement("input");
+          radio.type = "radio";
+          radio.id = useriddeviceid;
+          radio.setAttribute("name" , "device");
+          radio.setAttribute("onclick", "selectDevice()");     
+          label.appendChild(radio);
+          label.appendChild(document.createTextNode(useriddeviceid));
+          return label;
+        }
+        var userdevice_button;
+        var i;
+        for (i = 0; i < user.length; i++) {
+          userdevice_button = makeRadioButton("device", user[i] + "/" + device[i], user[i] + "/" + device[i]);
+          radio_home.appendChild(userdevice_button);
+          radio_home.innerHTML += "<br>";
+       }
+      </script>

@edautz
Copy link
Contributor Author

edautz commented Jul 6, 2018

Testen my version on a win10 machine using Chrome and IE11. Runs without problems.

I think your content of api/0/last is different and my program is not capable to handle that. Can you provide your version? So I can solve that.

@jpmens
Copy link
Member

jpmens commented Jul 6, 2018

It cannot be different if you are using a rather current version of Recorder.

Also: please address the issue of index.html being in the wrong directory; it should be in docroot/ and not in the root of the project (as mentioned above already).

I probably won't be able to work on this for the next weeks.

@edautz
Copy link
Contributor Author

edautz commented Jul 6, 2018

Index.html location fixed.

I am using one of the lastest version of ot recorder, version 0.7.6-16-g7a1562e24e.

But I am not using the owntrack app for the location information, but iCloud information and a custom build car tracking device with a gps reciever. The mapping and translation of the incoming data of these devices is done with Nodered and the results are fed with mqtt to ot-recorder.

I had to experiment with datafields to determine wich fields were nessecary for OT recorder, I use the user, device and fulldate fields in this order to extract the user and device information in the index.html file.

If your fields are in another order or other fields are between them, my mechanism will not work and this could result in an empty user/device list.

So for me it make sense to get a copy of your api/0/last data. So I can check this, and if so, modify my dataorder and index.html that this complies to your standard.

@lennardk
Copy link

@edautz fwiw, the userlist works for me if I use 'username', 'device' and 'ghash' for finding the string positions.
That being said, it doesn't feel very robust to depend on these fields being all present and in this specific order in the message. Wouldn't it be easier to just pull the response through a JSON parser and directly access the fields?

Discovering users and devices independent of the /api/0/last datafields order.
@edautz
Copy link
Contributor Author

edautz commented Jul 19, 2018

Made the index.html file user and device discovery independent of the order of the fields. Just the fields user and device has to be present in the /api/0/last file.

@lennardk
Copy link

lennardk commented Jul 19, 2018

for me, the field is "username", but the current code does match that so it's fine. You just have a typo on line 61; if I understand what you're doing correctly:

-          if (endposdev < endposus) biggest = endpondus;
+          if (endposdev < endposus) biggest = endposus;

@edautz
Copy link
Contributor Author

edautz commented Jul 19, 2018

Fixed typo. Can you confirm the standard name of the field is username instead of user?

I got my information of the fields from:

https://owntracks.org/booklet/tech/json/

There its called user.

@lennardk
Copy link

I think ot-recorder doesn't copy the owntracks fields directly to its API. Mainly looking at this part of the README which lists it as uersname: https://github.com/owntracks/recorder#last-position-of-a-particular-user

@edautz
Copy link
Contributor Author

edautz commented Jul 19, 2018

Did a double check. User instead of username in the code shouldn’t be an issue because a match on user is also a match on username and the position mapping is correct.

If you and @jpmens agree. I leave the code as it is.

@jpmens
Copy link
Member

jpmens commented Jul 20, 2018

@edautz it's not a case of agreeing; the code should work and be robust. If you are matching strings (which it sounds like when I read match on user is also a match on username) then the code is not robust. I think @lennardk said "JSON parser" which is what must happen to make it robust.

@jpmens
Copy link
Member

jpmens commented Jul 20, 2018

I'm on mobile which is hard on the eyes, and hard to look at from here. I see this at the top:

beginposus = urldata.indexOf("user", stringpointer);

That will not work.

Changes search for user to username
@edautz
Copy link
Contributor Author

edautz commented Jul 20, 2018

I think the code is robust now and working now for all kinds of field order. I choose to use plain javascript and only use a third party calender tool, because not reinvent the wheel. I change the search for user to username for clearness. I am only a hobby programmer putting a lot of effort to make a more usefull homepage.

Copy link
Member

@jpmens jpmens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@edautz be assured your effort is highly appreciated as is your involvement with our small project!

I have marked the areas I think could use a bit of improvement.

document.getElementById('error3').innerHTML = "";
//var dataURI = "http://192.168.2.113:8083/api/0/last";
var dataURI = "/api/0/last";
var Httpreq = new XMLHttpRequest(); // a new request
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think error-handling is missing here

var index = 0;
// get users and devices
while (stringpointer < urldata.length) {
beginposus = urldata.indexOf("username", stringpointer);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still matching strings. Please see about using the JavaScript JSON functions to parse the data which is returned from the Recorder's API

endposus = urldata.indexOf('"', beginposus+13);
beginposdev = urldata.indexOf("device", stringpointer);
endposdev = urldata.indexOf('"', beginposdev+9);
if (endposdev < endposus) biggest = endposus;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole block looks brittle

<br>

<script>
// build calender two formats one for mobile Safari voor iPad and iPhone and one for the rest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why must Safari on iOS be different to "the rest"?

Calender = makeCalenderidevice("Date from:", "fromdatetime", "&nbsp;");
calender_home.appendChild(Calender);
calender_home.innerHTML += "<br>";
Calender = makeCalenderidevice("Date to:", "todatetime", "&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  ? really?

var urlpart3 = "&format=";
var urlpart4 = "&from=";
var urlpart5 = "&to=";
var url = urlpart1 + usermap + urlpart2 + devicemap + urlpart3 + maptype + urlpart4 + urlfromdate + urlpart5 + urltodate;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there not some function which will escape these values for you?

@edautz
Copy link
Contributor Author

edautz commented Jul 25, 2018

I like your project very much, and use it several weeks now with great pleasure as viewing part of my car and telephone tracking system.

I regret to see that my effort doesn’t meet your standards.

I am afraid I can’t comply to your request because as a hobby programmer I don’t have skills to make your requested changes. Maybe a more skilled programmer can make the required changes to comply to your code quality standards.

I only can say that this new homepage runs stable without any problems in my setup.

@jpmens
Copy link
Member

jpmens commented Jan 19, 2019

@edautz thank you for your work and your attempted contribution; very much appreciated.
I will now close this in favor of a possible solution via #284

@jpmens jpmens closed this Jan 19, 2019
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.

3 participants