Displaying weather data from an Arduino

Question

Please review my code, for performance and readability. 3 issues I am facing are

1. Uncaught Exception : out of memory (in firefox)
2. randomly Failed to load resource: The request timed out.
3. Even if the json data is pulled instantly looks like seInterval is adding lag to display the result both in tabular data display or google gauge

---

 var url = 'http://www.example.com/json';
 function removeTable(id)
 {
 var tbl = document.getElementById(id);
 if (tbl) tbl.parentNode.removeChild(tbl);
 }
 document.addEventListener('DOMContentLoaded', function ()
 {
 // do not display gauge on mobile devices
 if (jscd.mobile)
 {
 removeTable("gaugeTable")
 document.getElementById("gaugeTable").innerHTML = "";
 }
 }, false);
 (function (window)
 {
 {
 var unknown = '-';
 // browser
 var nVer = navigator.appVersion;
 // mobile version
 var mobile = /Mobile|mini|Fennec|Android|iP(ad|od|hone)/.test(nVer);
 }
 window.jscd = {
 mobile: mobile,
 };
 }(this));
 $().ready(function ()
 {
 setInterval(function ()
 {
 jQuery.support.cors = true;
 // do not pull and display non-mobile devices.
 if (jscd.mobile)
 {
 $.ajax(
 {
 url: url,
 data:
 {
 format: 'json'
 },
 error: function (jqXHR, textStatus, errorThrown)
 {
 alert(textStatus + ': ' + errorThrown);
 },
 dataType: 'json',
 crossDomain: true,
 success: function (sensorData)
 {
 var tempHumidData = sensorData.arduino;
 renderHTML(tempHumidData);
 },
 type: 'GET'
 });
 }
 }, 5000);
 });
 function renderHTML(data)
 {
 if (data)
 {
 removeTable("pullingDataMsg")
 var sensorDataContainer = document.getElementById("displaySensorData");
 var htmlString = "<table id='headTable' style='width:100%'><tr><th>Location</th><th>Temperature</th><th>Humidity</th></tr>";
 for (i = 0; i < data.length; i++)
 {
 htmlString += "<tr align='center'><td>" + data[i].location + "</td><td>" + data[i].temperatureInC + "&deg;C/ " + data[i].temperatureInF + "&deg;F </td> <td> " + data[i].humidity + "%</td></tr>";
 }
 htmlString += "</table>"
 sensorDataContainer.innerHTML = htmlString;
 }
 }
 //------------- Google Gauge -------------------//
 // global variables
 var chart, humidityChart, data, humidityData;
 // maximum value for the gauge
 var max_gauge_value = 70;
 // name of the gauge
 var gauge_name = 'Temperature';
 var outTemp, drwngRomTemp, outHumidity, drwRomHumid;
 var hoptions = {
 animation:
 {
 duration: 1000,
 easing: 'inAndOut'
 },
 min: 0,
 max: 100,
 redFrom: 90,
 redTo: 100,
 yellowFrom: 75,
 yellowTo: 90,
 minorTicks: 5
 };
 // load the google gauge visualization
 google.load('visualization', '1',
 {
 'packages': ['gauge']
 });
 google.setOnLoadCallback(initChart);
 // display the data
 function displayData(outTemp, drwngRomTemp, outHumidity, drwRomHumid)
 {
 chart.draw(data, options);
 data.setValue(0, 0, gauge_name);
 data.setValue(0, 1, outTemp);
 data.setValue(1, 0, gauge_name);
 data.setValue(1, 1, drwngRomTemp);
 humidityChart.draw(humidityData, hoptions);
 humidityData.setValue(0, 0, "Humidity");
 humidityData.setValue(0, 1, outHumidity);
 humidityData.setValue(1, 0, "Humidity");
 humidityData.setValue(1, 1, drwRomHumid);
 removeTable("pullingDataMsg")
 }
 // load the data
 function loadData()
 {
 // get the data from arduino
 $.ajax(
 {
 url: url,
 data:
 {
 format: 'json'
 },
 error: function (jqXHR, textStatus, errorThrown)
 {
 alert(textStatus + ': ' + errorThrown);
 },
 dataType: 'json',
 crossDomain: true,
 success: function (sensorData)
 {
 // set the sensor data pulled from arduino to global variable sensorData
 // get the data point
 outTemp = sensorData.arduino[0].temperatureInC;
 drwngRomTemp = sensorData.arduino[1].temperatureInC;
 outHumidity = sensorData.arduino[0].humidity;
 drwRomHumid = sensorData.arduino[1].humidity;
 displayData(outTemp, drwngRomTemp, outHumidity, drwRomHumid);
 },
 type: 'GET'
 });
 }
 // initialize the chart
 function initChart()
 {
 var initTempData = {
 "cols": [
 {
 "id": "",
 "label": "Label",
 "pattern": "",
 "type": "string"
 },
 {
 "id": "",
 "label": "Temperature",
 "pattern": "",
 "type": "number"
 }],
 "rows": [
 {
 "c": [
 {
 "v": "Temperature",
 "f": null
 },
 {
 "v": -20,
 "f": null
 }]
 },
 {
 "c": [
 {
 "v": "Temperature",
 "f": null
 },
 {
 "v": -20,
 "f": null
 }]
 }]
 };
 data = new google.visualization.DataTable(initTempData);
 options = {
 greenFrom: 10,
 greenTo: 29,
 redFrom: 41,
 redTo: 70,
 yellowFrom: 30,
 yellowTo: 40,
 majorTicks: ['-10', '0', '10', '20', '30', '40', '50', '60', ''],
 minorTicks: 5,
 animation:
 {
 duration: 1000,
 easing: 'inAndOut'
 },
 min: -20,
 max: 70,
 greenColor: '#CCFFCC',
 yellowColor: '#FFFFCC',
 redColor: '#F78181'
 };
 chart = new google.visualization.Gauge(document.getElementById('gauge_div'));
 var initHumidData = {
 "cols": [
 {
 "id": "",
 "label": "Label",
 "pattern": "",
 "type": "string"
 },
 {
 "id": "",
 "label": "Humidity",
 "pattern": "",
 "type": "number"
 }],
 "rows": [
 {
 "c": [
 {
 "v": "Humidity",
 "f": null
 },
 {
 "v": 0,
 "f": null
 }]
 },
 {
 "c": [
 {
 "v": "Humidity",
 "f": null
 },
 {
 "v": 0,
 "f": null
 }]
 }]
 };
 humidityData = new google.visualization.DataTable(initHumidData);
 humidityChart = new google.visualization.Gauge(document.getElementById('humidity_gauge_div'));
 loadData();
 // load new data every 5 seconds
 setInterval('loadData()', 5000);
 }

This is my first javascript code review, I am newbie into learning javascript, so I have arduino MCU hooked with Sensors that give temperature and humidity data, arduino MCU runs web server passing json data which I am calling by ajax in the above code. I am displaying google gauge on non-mobile platform and textual table in mobile devices. the live preview can be seen here:

I want to know how to write efficient/ performance intuitive javascript code.

Answer 1

• I don't see any reason to use tables. Use CSS and style the HTML accordingly. ul displayed inline can work wonders.

• Also, even if you're a beginner you may want to consider mustache . It's something that I'm sure will come in handy to you anyway.

• You can keep the id of the elements instead of calling document.getElementById every time. This goes for sensorDataContainer, gaugeTable (see below about this), displaySensorData, gauge_div, pullingDataMsg. Initialize the global variables at the beginning and re-use them.

• You're actually removing elements. This is not nice, and in my opinion not done properly. If your problem is only screen or display size, I think the best way is to use a CSS media query so that for smaller screens the gauge bar is not displayed. That's better than adding it and then removing it. This is not exclusive, you can use both CSS and JS for that, but keep in mind that user agents are a mess. For example, Nokia Lumia phones have a user agent string like this: Mozilla/5.0 (WM 10.0; Android <Android Version>; <Device Manufacturer>; <Device Model>) AppleWebKit/<WebKit Rev> (KHTML, like Gecko) Chrome/<Chrome Rev> Mobile Safari/<WebKit Rev> Edge/<EdgeHTML Rev>.<Windows Build>. I understand you copied and pasted that from here, but please consider the use case when you copy code from the internet.

Also, even if you really wanted to use it, this:

 (function (window)
 {
 {
 var unknown = '-';
 // browser
 var nVer = navigator.appVersion;
 // mobile version
 var mobile = /Mobile|mini|Fennec|Android|iP(ad|od|hone)/.test(nVer);
 }
 window.jscd = {
 mobile: mobile,
 };
 }(this));

Could be something like this:

 (function (window)
 {
 var mobile = /Mobile|mini|Fennec|Android|iP(ad|od|hone)/.test(navigator.appVersion);
 window.jscd = { mobile: mobile };
 }(this));

• var unknown is never used.

• I wouldn't user alert in any case, especially if you have something that you may want to let running for some time with a setInterval . If you have an error and come back to the screen after a while, you're going to have to do quite a lot of clicking to dismiss those alerts. If you have a problem retrieving data, display a proper error on the page content.

• You're using setInterval(function () {...}, 5000); and setInterval('loadData()', 5000); There's a difference in passing a string and a function as argument to setInterval, are you sure that's what you want?

• This means you have two functions running every 5 seconds. They do apparently similar tasks but it's not really clear what the purpose of each one is. Since there is quite a lot to fix, my advice is to remove one of the two, focus on that one and only on that one (no browser detection or any extras). See that it works properly and it's written correctly, and only after that try and expand it. For debugging purposes (and only for that), you can use console.log to write messages to the browser console.

• Another suggestion is to understand the code that you find before you use it. For example, why do you have jQuery.support.cors in your code and why do you need to set it to true every 5 seconds?

• Ciasto piekarz: Thank you very much for input, I removed alert and used console.log instead. Thats i was thinking to at first place. I tried setInterval('loadData', 10000); instead of setInterval('loadData()', 10000); it did not loaded google gauge at all. Also can you please give example of ul with inline-block ?
• ChatterOne: The difference is not (only) in having the parenthesis or not, it's about using it as a string or not, you should remove the single quotes, read carefully the link I posted about that. About the ul there are many answer here. You can have a look at this e.g. stackoverflow.com/questions/11300538/…