Skip to content

Commit

Permalink
Unify the handling of no-data entries
Browse files Browse the repository at this point in the history
- parser will return 'null' for no-data entries
- xviz stream buffer will handle the 'null' value appropropriately
- log slice will respect the 'null' value as a no-data entry and not keep
  searching
  • Loading branch information
twojtasz committed Aug 15, 2019
1 parent ad701c5 commit 81b5e4b
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 15 deletions.
10 changes: 10 additions & 0 deletions modules/parser/src/parsers/parse-timeslice-data-v2.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ function parseStreamSets(streamSets, timestamp, convertPrimitive) {
const timeSeries = [];
const futures = {};
const uiPrimitives = {};
const noDataStreams = [];

for (const streamSet of streamSets) {
Object.assign(poses, streamSet.poses);
Expand All @@ -110,6 +111,10 @@ function parseStreamSets(streamSets, timestamp, convertPrimitive) {
timeSeries.push(...streamSet.time_series);
}
}

if (streamSet.no_data_streams && noDataStreams) {
noDataStreams.push(...streamSet.no_data_streams);
}
}

Object.keys(poses)
Expand Down Expand Up @@ -156,6 +161,11 @@ function parseStreamSets(streamSets, timestamp, convertPrimitive) {
);
});

if (noDataStreams.length) {
// Explicitly set to null and the stream buffer will do the right thing
noDataStreams.forEach(stream => (newStreams[stream] = null));
}

return newStreams;
}
/* eslint-enable max-statements */
11 changes: 10 additions & 1 deletion modules/parser/src/synchronizers/log-slice.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,11 @@ export default class LogSlice {
// get data if we don't already have that stream && it is not filtered.
streamsByReverseTime.forEach(streams => {
for (const streamName in streams) {
if (!this.streams[streamName] && this._includeStream(filter, streamName)) {
if (
this.streams[streamName] !== null && // Explicit no data entry
!this.streams[streamName] && // undefined means it has not been seen so keep looking for valid entry
this._includeStream(filter, streamName)
) {
this.addStreamDatum(streams[streamName], streamName, lookAheadMs, this);
}
}
Expand All @@ -127,6 +131,11 @@ export default class LogSlice {
addStreamDatum(datum, streamName, lookAheadMs) {
this.streams[streamName] = datum;

// Handle the no data case
if (!datum) {
return;
}

this.setLabelsOnXVIZObjects(datum.labels);

const {features = [], lookAheads = [], variable, pointCloud = null} = datum;
Expand Down
3 changes: 1 addition & 2 deletions modules/parser/src/synchronizers/xviz-stream-buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -383,11 +383,10 @@ export default class XVIZStreamBuffer {
});

for (const streamName in timeslice.streams) {
let value = timeslice.streams[streamName];
const value = timeslice.streams[streamName];
if (value === null) {
// Explicitly delete a stream
delete timesliceAtInsertPosition.streams[streamName];
value = undefined;
}
streams[streamName][index] = value;
}
Expand Down
36 changes: 36 additions & 0 deletions test/modules/parser/parsers/parse-xviz-message-sync.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,42 @@ tape('parseXVIZData futures timeslice v1', t => {
t.end();
});

tape('parseXVIZData state_update, PERSISTENT', t => {
resetXVIZConfigAndSettings();
setXVIZConfig({currentMajorVersion: 2, ALLOW_MISSING_PRIMARY_POSE: true});

const persistentMsg = {...TestTimesliceMessageV2};
persistentMsg.update_type = 'PERSISTENT';

const result = parseXVIZData(persistentMsg, {v2Type: 'state_update'});
t.equals(result.type, XVIZ_MESSAGE_TYPE.TIMESLICE, 'Message type set for timeslice');
t.equal(result.updateType, 'PERSISTENT', 'XVIZ update type is parsed');
t.equals(result.timestamp, TestTimesliceMessageV2.updates[0].timestamp, 'Message timestamp set');

const feature = result.streams['/test/stream'].features[0];
t.equal(feature.type, 'point', 'feature has type point');
t.deepEquals(Array.from(feature.points), [1000, 1000, 200], 'feature has type point');

t.end();
});

tape('parseXVIZData state_update, no_data_streams', t => {
resetXVIZConfigAndSettings();
setXVIZConfig({currentMajorVersion: 2, ALLOW_MISSING_PRIMARY_POSE: true});

const noDataStreamMsg = {...TestTimesliceMessageV2};
noDataStreamMsg.updates[0].no_data_streams = ['/no-data-stream'];

const result = parseXVIZData(noDataStreamMsg, {v2Type: 'state_update'});
t.equals(result.type, XVIZ_MESSAGE_TYPE.TIMESLICE, 'Message type set for timeslice');
t.equal(result.updateType, 'COMPLETE', 'XVIZ update type is parsed');
t.equals(result.timestamp, TestTimesliceMessageV2.updates[0].timestamp, 'Message timestamp set');

t.equal(result.streams['/no-data-stream'], null, 'no_data_stream marked as null');

t.end();
});

tape('parseXVIZMessageSync', t => {
resetXVIZConfigAndSettings();
setXVIZConfig({currentMajorVersion: 2});
Expand Down
22 changes: 11 additions & 11 deletions test/modules/parser/synchronizers/stream-synchronizer.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,8 @@ tape('StreamSynchronizer#correct lookup with empty entries (explicit no-data)',
// start both with no-data entry
timestamp: 90,
streams: {
stream1: {},
stream2: {}
stream1: null,
stream2: null
}
},
{
Expand Down Expand Up @@ -197,8 +197,8 @@ tape('StreamSynchronizer#correct lookup with empty entries (explicit no-data)',
// add no-data entry for both
timestamp: 103,
streams: {
stream1: {},
stream2: {}
stream1: null,
stream2: null
}
},
{
Expand All @@ -213,8 +213,8 @@ tape('StreamSynchronizer#correct lookup with empty entries (explicit no-data)',
// no-data entry for both
timestamp: 115,
streams: {
stream1: {},
stream2: {}
stream1: null,
stream2: null
}
},
{
Expand All @@ -235,14 +235,14 @@ tape('StreamSynchronizer#correct lookup with empty entries (explicit no-data)',
// empty entry for stream2
timestamp: 122,
streams: {
stream2: {}
stream2: null
}
}
];

const streamSynchronizer = new StreamSynchronizer(STREAMS_WITH_NO_DATA_ENTRIES);

// Test a time before any valid entries
// Test a time before any valid entries and with no entries within TIME_WINDOW
streamSynchronizer.setTime(99);
let data = streamSynchronizer.getLogSlice();
t.equals(data.streams.stream1, undefined, 'stream1 is undefined at time 99');
Expand All @@ -260,11 +260,11 @@ tape('StreamSynchronizer#correct lookup with empty entries (explicit no-data)',
t.equals(data.streams.stream1.value, 1.5, 'stream1 is 1.5 at time 102');
t.equals(data.streams.stream2.value, 10, 'stream2 is 10 at time 102');

// Test a time that has no-data entry for both streams
// Test a time that has no-data entry for both streams within the time-window
streamSynchronizer.setTime(103);
data = streamSynchronizer.getLogSlice();
t.equals(data.streams.stream1.value, undefined, 'stream1 is undefined at time 103');
t.equals(data.streams.stream2.value, undefined, 'stream2 is undefined at time 103');
t.equals(data.streams.stream1, null, 'stream1 is explicitly no-data at time 103');
t.equals(data.streams.stream2, null, 'stream2 is explicitly no-data at time 103');

// Test a both streams picked up from same entry
streamSynchronizer.setTime(110);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ test('XVIZStreamBuffer#insert, getStreams', t => {

t.deepEquals(
xvizStreamBuffer.getStreams(),
{A: [1.1, 2.2, 3, 4, 5], B: [-1], C: [1]},
{A: [1.1, 2.2, 3, 4, 5], B: [null, -1], C: [1]},
'getStreams returns correct result'
);

Expand Down

0 comments on commit 81b5e4b

Please sign in to comment.