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

handleSessionTime #37

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 106 additions & 2 deletions src/JavaScript/SCORM_API_wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,13 @@ pipwerks.SCORM = { //Define the SCORM object
version: null, //Store SCORM version.
handleCompletionStatus: true, //Whether or not the wrapper should automatically handle the initial completion status
handleExitMode: true, //Whether or not the wrapper should automatically handle the exit mode
handleSessionTime: true, //Whether or not the wrapper should automatically handle the session time
Copy link
Contributor

Choose a reason for hiding this comment

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

trailing space on the end of this line (and on the majority of lines you have added)

Copy link
Contributor

Choose a reason for hiding this comment

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

and just a thought but defaulting this to true could cause problems for anyone who goes to update to the latest version in their project... if they don't realised this functionality has been added then they run the risk of having two separate bits of code trying to set the session_time... I'll defer to @pipwerks as to what he thinks the default should be.

Copy link
Author

Choose a reason for hiding this comment

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

Changed to false, I do agree this way will allow a more "retrocompatible" upgrade.

API: { handle: null,
isFound: false }, //Create API child object
connection: { isActive: false }, //Create connection child object
data: { completionStatus: null,
exitStatus: null }, //Create data child object
exitStatus: null,
dtmInitialized: null}, //Create data child object
debug: {} //Create debug child object
};

Expand Down Expand Up @@ -251,6 +253,9 @@ pipwerks.SCORM.connection.initialize = function(){

trace("connection.initialize called.");

//set init date-time
scorm.data.dtmInitialized = new Date();

if(!scorm.connection.isActive){

var API = scorm.API.getHandle(),
Expand Down Expand Up @@ -356,6 +361,7 @@ pipwerks.SCORM.connection.terminate = function(){
scorm = pipwerks.SCORM,
exitStatus = scorm.data.exitStatus,
completionStatus = scorm.data.completionStatus,
dtmInitialized = scorm.data.dtmInitialized,
trace = pipwerks.UTILS.trace,
makeBoolean = pipwerks.UTILS.StringToBoolean,
debug = scorm.debug,
Expand All @@ -369,7 +375,28 @@ pipwerks.SCORM.connection.terminate = function(){

if(API){

if(scorm.handleExitMode && !exitStatus){
if(scorm.handleSessionTime){

var dtm = new Date();

//in the next line you substract the time recorded when initialising
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: 'substract' should be 'subtract'

//the connection from the present time.
var n = dtm.getTime() - dtmInitialized.getTime();
switch(scorm.version){

//the time format is different on scorm 1.2 or 2004, so we use
//different conversions depending on the case
case "1.2" :
this.set("cmi.core.session_time",pipwerks.UTILS.MillisecondsToCMIDuration(n));
break;
case "2004":
this.set("cmi.session_time",pipwerks.UTILS.centisecsToISODuration(Math.floor(n/10)));
break;
}

}

if(scorm.handleExitMode && !exitStatus){

if(completionStatus !== "completed" && completionStatus !== "passed"){

Expand Down Expand Up @@ -841,3 +868,80 @@ pipwerks.UTILS.trace = function(msg){

}
};


/* -------------------------------------------------------------------------
pipwerks.UTILS.MillisecondsToCMIDuration()
Converts time to scorm 1.2 time format

Parameters: n (number)
Return: String
---------------------------------------------------------------------------- */

pipwerks.UTILS.MillisecondsToCMIDuration = function(n){
Copy link
Contributor

Choose a reason for hiding this comment

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

it's just my opinion but I think the function name could be shortened to msToCMIDuration and the intention of it would still be clear. similarly csToISODuration still seems like a clear name to me.

Copy link
Author

Choose a reason for hiding this comment

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

totally agree ;)


//Convert duration from milliseconds to 0000:00:00.00 format
Copy link
Contributor

Choose a reason for hiding this comment

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

could you move this comment to the main comment block above?

n = (!n || n<0)? 0 : n; //default value and force positive duration
Copy link
Contributor

Choose a reason for hiding this comment

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

could you add some spacing around the operators for consistency with the rest of the code?

var hms = "";
var dtm = new Date(); dtm.setTime(n);
var h = "0" + Math.floor(n / 3600000);
var m = "0" + dtm.getMinutes();
var s = "0" + dtm.getSeconds();
hms = h.substr(h.length-2)+":"+m.substr(m.length-2)+":";
Copy link
Contributor

Choose a reason for hiding this comment

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

could you add some spacing around the operators for consistency (and readability)

hms += s.substr(s.length-2);
return hms
Copy link
Contributor

Choose a reason for hiding this comment

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

missing semicolon


};


/* -------------------------------------------------------------------------
pipwerks.UTILS.centisecsToISODuration()
Converts time to scorm 2004 time format

Parameters: n (number)
Return: String
---------------------------------------------------------------------------- */

pipwerks.UTILS.centisecsToISODuration = function(n){

// Note: SCORM and IEEE 1484.11.1 require centisec precision
// Months calculated by approximation based on average number
// of days over 4 years (365*4+1), not counting the extra day
// every 1000 years. If a reference date was available,
// the calculation could be more precise, but becomes complex,
// since the exact result depends on where the reference date
// falls within the period (e.g. beginning, end or ???)
// 1 year ~ (365*4+1)/4*60*60*24*100 = 3155760000 centiseconds
// 1 month ~ (365*4+1)/48*60*60*24*100 = 262980000 centiseconds
// 1 day = 8640000 centiseconds
// 1 hour = 360000 centiseconds
// 1 minute = 6000 centiseconds
n = Math.max(n,0); // there is no such thing as a negative duration
var str = "P";
var nCs = n;
// Next set of operations uses whole seconds
var nY = Math.floor(nCs / 3155760000);
nCs -= nY * 3155760000;
var nM = Math.floor(nCs / 262980000);
nCs -= nM * 262980000;
var nD = Math.floor(nCs / 8640000);
nCs -= nD * 8640000;
var nH = Math.floor(nCs / 360000);
nCs -= nH * 360000;
var nMin = Math.floor(nCs /6000);
nCs -= nMin * 6000
Copy link
Contributor

Choose a reason for hiding this comment

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

missing semi colon

// Now we can construct string
if (nY > 0) str += nY + "Y";
Copy link
Contributor

Choose a reason for hiding this comment

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

I love that you've assumed someone may have spent years in the same session - that would be one dedicated learner ;-) I didn't even bother going above days in my implementation!!

Copy link
Author

Choose a reason for hiding this comment

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

We are not gonna set the limits of the eLearning ;)

if (nM > 0) str += nM + "M";
if (nD > 0) str += nD + "D";
if ((nH > 0) || (nMin > 0) || (nCs > 0)) {
str += "T";
if (nH > 0) str += nH + "H";
if (nMin > 0) str += nMin + "M";
if (nCs > 0) str += (nCs / 100) + "S";
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation is a bit off here

}
if (str == "P") str = "PT0H0M0S";
// technically PT0S should do but SCORM test suite assumes longer form.
return str;

};
16 changes: 16 additions & 0 deletions test/JavaScript/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width">
<title>SCORM_API_wrapper.js - tests</title>
<link rel="stylesheet" href="https://code.jquery.com/qunit/qunit-2.4.1.css">
</head>
<body>
<div id="qunit"></div>
<div id="qunit-fixture"></div>
<script src="../../src/JavaScript/SCORM_API_wrapper.js"></script>
<script src="https://code.jquery.com/qunit/qunit-2.4.1.js"></script>
<script src="test.js"></script>
</body>
</html>
31 changes: 31 additions & 0 deletions test/JavaScript/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
QUnit.test("centisecsToISODuration", function (assert){

var t1 = 1513937992632;
var t2 = 1513938095752;
var d = t2 - t1;

assert.equal(pipwerks.UTILS.centisecsToISODuration(d), 'PT17M11.2S');
Copy link
Contributor

Choose a reason for hiding this comment

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

could you add descriptions for your tests? e.g.

assert.equal(pipwerks.UTILS.centisecsToISODuration(null), 'PT0H0M0S', 'Check null is correctly handled');

assert.equal(pipwerks.UTILS.centisecsToISODuration(), 'PT0H0M0S');
assert.equal(pipwerks.UTILS.centisecsToISODuration(undefined), 'PT0H0M0S');
assert.equal(pipwerks.UTILS.centisecsToISODuration(null), 'PT0H0M0S');
assert.equal(pipwerks.UTILS.centisecsToISODuration(''), 'PT0H0M0S');
assert.equal(pipwerks.UTILS.centisecsToISODuration(0), 'PT0H0M0S');
assert.equal(pipwerks.UTILS.centisecsToISODuration(-1), 'PT0H0M0S');

});

QUnit.test("MillisecondsToCMIDuration", function (assert) {

var t1 = 1513937992632;
var t2 = 1513938095752;
var d = t2 - t1;

assert.equal(pipwerks.UTILS.MillisecondsToCMIDuration(d), '00:01:43');
assert.equal(pipwerks.UTILS.MillisecondsToCMIDuration(), '00:00:00');
assert.equal(pipwerks.UTILS.MillisecondsToCMIDuration(undefined), '00:00:00');
assert.equal(pipwerks.UTILS.MillisecondsToCMIDuration(null), '00:00:00');
assert.equal(pipwerks.UTILS.MillisecondsToCMIDuration(''), '00:00:00');
assert.equal(pipwerks.UTILS.MillisecondsToCMIDuration(0), '00:00:00');
assert.equal(pipwerks.UTILS.MillisecondsToCMIDuration(-1), '00:00:00');

});