sensors-logging: Fix calculation of duration intervals to consider milliseconds #739

rafaellehmkuhl · 2024-02-16T21:53:47Z

Examples of the logs before and after:

Dialogue: 0,0:0:111.00,0:0:112.00,
Dialogue: 0,0:0:112.00,0:0:113.00,
Dialogue: 0,0:0:114.00,0:0:115.00,
Dialogue: 0,0:0:117.00,0:0:118.00,

Dialogue: 0,00:02:00.79,00:02:01.79
Dialogue: 0,00:02:01.79,00:02:02.80
Dialogue: 0,00:02:02.80,00:02:03.81
Dialogue: 0,00:02:03.81,00:02:04.79

Fix #625

src/libs/sensors-logging.ts

…lliseconds

patrickelectric · 2024-02-22T17:54:02Z

src/libs/sensors-logging.ts

-      const secondsStart = Math.round(differenceInMilliseconds(new Date(logPoint.epoch), new Date(videoStartEpoch))/1000)
-      const secondsFinish = secondsStart + 1
+      const durationThisPoint = intervalToDuration({
+        start: new Date(videoStartEpoch),
+        end: new Date(logPoint.epoch),
+      })
+      const durationHoursThisPoint = (durationThisPoint.hours?.toFixed(0) ?? '0').padStart(2, '0')
+      const durationMinutesThisPoint = (durationThisPoint.minutes?.toFixed(0) ?? '0').padStart(2, '0')
+      const durationSecondsThisPoint = (durationThisPoint.seconds?.toFixed(0) ?? '0').padStart(2, '0')
+
+      const durationNextPoint = intervalToDuration({
+        start: new Date(videoStartEpoch),
+        end: new Date(log[index + 1].epoch ?? 0),
+      })
+      const durationHoursNextPoint = (durationNextPoint.hours?.toFixed(0) ?? '0').padStart(2, '0')
+      const durationMinutesNextPoint = (durationNextPoint.minutes?.toFixed(0) ?? '0').padStart(2, '0')
+      const durationSecondsNextPoint = (durationNextPoint.seconds?.toFixed(0) ?? '0').padStart(2, '0')
+
+      const roundedMillisThisPoint = differenceInSeconds(new Date(logPoint.epoch), new Date(videoStartEpoch)) * 1000
+      const millisThisPoint = differenceInMilliseconds(new Date(logPoint.epoch), new Date(videoStartEpoch))
+      const remainingMillisThisPoint = millisThisPoint - roundedMillisThisPoint
+      const remainingCentisThisPoint = Math.floor(remainingMillisThisPoint / 10).toString().padStart(2, '0')
+
+      const roundedMillisNextPoint = differenceInSeconds(new Date(log[index + 1].epoch), new Date(videoStartEpoch)) * 1000
+      const millisNextPoint = differenceInMilliseconds(new Date(log[index + 1].epoch), new Date(videoStartEpoch))
+      const remainingMillisNextPoint = millisNextPoint - roundedMillisNextPoint
+      const remainingCentisNextPoint = Math.floor(remainingMillisNextPoint / 10).toString().padStart(2, '0')


I would recommend using Date format function to do the dirty job.

Suggested change

const secondsStart = Math.round(differenceInMilliseconds(new Date(logPoint.epoch), new Date(videoStartEpoch))/1000)

const secondsFinish = secondsStart + 1

const durationThisPoint = intervalToDuration({

start: new Date(videoStartEpoch),

end: new Date(logPoint.epoch),

})

const durationHoursThisPoint = (durationThisPoint.hours?.toFixed(0) ?? '0').padStart(2, '0')

const durationMinutesThisPoint = (durationThisPoint.minutes?.toFixed(0) ?? '0').padStart(2, '0')

const durationSecondsThisPoint = (durationThisPoint.seconds?.toFixed(0) ?? '0').padStart(2, '0')

const durationNextPoint = intervalToDuration({

start: new Date(videoStartEpoch),

end: new Date(log[index + 1].epoch ?? 0),

})

const durationHoursNextPoint = (durationNextPoint.hours?.toFixed(0) ?? '0').padStart(2, '0')

const durationMinutesNextPoint = (durationNextPoint.minutes?.toFixed(0) ?? '0').padStart(2, '0')

const durationSecondsNextPoint = (durationNextPoint.seconds?.toFixed(0) ?? '0').padStart(2, '0')

const roundedMillisThisPoint = differenceInSeconds(new Date(logPoint.epoch), new Date(videoStartEpoch)) * 1000

const millisThisPoint = differenceInMilliseconds(new Date(logPoint.epoch), new Date(videoStartEpoch))

const remainingMillisThisPoint = millisThisPoint - roundedMillisThisPoint

const remainingCentisThisPoint = Math.floor(remainingMillisThisPoint / 10).toString().padStart(2, '0')

const roundedMillisNextPoint = differenceInSeconds(new Date(log[index + 1].epoch), new Date(videoStartEpoch)) * 1000

const millisNextPoint = differenceInMilliseconds(new Date(log[index + 1].epoch), new Date(videoStartEpoch))

const remainingMillisNextPoint = millisNextPoint - roundedMillisNextPoint

const remainingCentisNextPoint = Math.floor(remainingMillisNextPoint / 10).toString().padStart(2, '0')

const durationThisPoint = formatDuration(new Date(logPoint.epoch - videoStartEpoch))

const durationNextPoint = formatDuration(new Date(log[index + 1].epoch - videoStartEpoch))

const remainingCentisThisPoint = formatCentiseconds(new Date(logPoint.epoch))

const remainingCentisNextPoint = formatCentiseconds(new Date(log[index + 1].epoch))

let subtitleDataString1 = ''

let subtitleDataString2 = ''

let subtitleDataString3 = ''

let binToUse = 1

data.forEach((d) => {

if (binToUse === 1) {

subtitleDataString1 = subtitleDataString1.concat(`${d.name}: ${d.value} \\N`)

} else if (binToUse === 2) {

subtitleDataString2 = subtitleDataString2.concat(`${d.name}: ${d.value} \\N`)

} else if (binToUse === 3) {

subtitleDataString3 = subtitleDataString3.concat(`${d.name}: ${d.value} \\N`)

}

binToUse +=1

if (binToUse > 3) {

binToUse = 1

}

})

It took some time to figure out the correct way to configure the Date string parser, but the function are the following:

const formatDuration = (date: Date): string => { return date.toLocaleTimeString([], { hourCycle: 'h23', hour: '2-digit', minute: '2-digit', second: '2-digit', timeZone: 'UTC', }) } const formatCentiseconds = (date: Date): string => { const millis = date.getMilliseconds() return String(Math.floor(millis / 10)).padStart(2, '0') }

Have you tested this code?

Dialogue: 0,00:00:04.12,00:00:06.14,Default,,192,0,54,,Roll: -1.0 ° \NDepth: -7.360 m \NGPS status: No GPS \N
Dialogue: 0,00:00:04.12,00:00:06.14,Default,,768,0,54,,Pitch: 2.4 ° \NMode: MANUAL \NLatitude: 0.000000 ° \N
Dialogue: 0,00:00:04.12,00:00:06.14,Default,,1344,0,54,,Heading: -163.2 ° \NGPS satellites: 0 \NLongitude: 0.000000 ° \N
Dialogue: 0,00:00:06.14,00:00:06.12,Default,,192,0,54,,Roll: -0.9 ° \NDepth: -7.520 m \NGPS status: No GPS \N
Dialogue: 0,00:00:06.14,00:00:06.12,Default,,768,0,54,,Pitch: 2.4 ° \NMode: MANUAL \NLatitude: 0.000000 ° \N
Dialogue: 0,00:00:06.14,00:00:06.12,Default,,1344,0,54,,Heading: -163.3 ° \NGPS satellites: 0 \NLongitude: 0.000000 ° \N
Dialogue: 0,00:00:06.12,00:00:07.12,Default,,192,0,54,,Roll: -0.9 ° \NDepth: -7.520 m \NGPS status: No GPS \N
Dialogue: 0,00:00:06.12,00:00:07.12,Default,,768,0,54,,Pitch: 2.4 ° \NMode: MANUAL \NLatitude: 0.000000 ° \N
Dialogue: 0,00:00:06.12,00:00:07.12,Default,,1344,0,54,,Heading: -163.2 ° \NGPS satellites: 0 \NLongitude: 0.000000 ° \N
Dialogue: 0,00:00:07.12,00:00:08.12,Default,,192,0,54,,Roll: -0.9 ° \NDepth: -7.500 m \NGPS status: No GPS \N
Dialogue: 0,00:00:07.12,00:00:08.12,Default,,768,0,54,,Pitch: 2.4 ° \NMode: MANUAL \NLatitude: 0.000000 ° \N
Dialogue: 0,00:00:07.12,00:00:08.12,Default,,1344,0,54,,Heading: -163.2 ° \NGPS satellites: 0 \NLongitude: 0.000000 ° \N

Testing this code here, it is generating broke results (notice the second dialog block).

@patrickelectric if this bug does not become clear, and you want to investigate more, I think we can merge the original code to have the original bug fixed, and you can propose the code improvement afterwards. What do you think?

Check #776
It should provide the correct output:

patrickelectric · 2024-02-22T18:00:29Z

src/libs/sensors-logging.ts

+      const timeThis = `${durationHoursThisPoint}:${durationMinutesThisPoint}:${durationSecondsThisPoint}.${remainingCentisThisPoint}`
+      const timeNext = `${durationHoursNextPoint}:${durationMinutesNextPoint}:${durationSecondsNextPoint}.${remainingCentisNextPoint}`
+
+      assFile = assFile.concat(`\nDialogue: 0,${timeThis},${timeNext},Default,,${0.1*videoWidth},0,${0.05*videoHeight},,${subtitleDataString1}`)
+      assFile = assFile.concat(`\nDialogue: 0,${timeThis},${timeNext},Default,,${0.4*videoWidth},0,${0.05*videoHeight},,${subtitleDataString2}`)
+      assFile = assFile.concat(`\nDialogue: 0,${timeThis},${timeNext},Default,,${0.7*videoWidth},0,${0.05*videoHeight},,${subtitleDataString3}`)


And following my previous comment:

Suggested change

const timeThis = `${durationHoursThisPoint}:${durationMinutesThisPoint}:${durationSecondsThisPoint}.${remainingCentisThisPoint}`

const timeNext = `${durationHoursNextPoint}:${durationMinutesNextPoint}:${durationSecondsNextPoint}.${remainingCentisNextPoint}`

assFile = assFile.concat(`\nDialogue: 0,${timeThis},${timeNext},Default,,${0.1*videoWidth},0,${0.05*videoHeight},,${subtitleDataString1}`)

assFile = assFile.concat(`\nDialogue: 0,${timeThis},${timeNext},Default,,${0.4*videoWidth},0,${0.05*videoHeight},,${subtitleDataString2}`)

assFile = assFile.concat(`\nDialogue: 0,${timeThis},${timeNext},Default,,${0.7*videoWidth},0,${0.05*videoHeight},,${subtitleDataString3}`)

const timeThis = `${durationThisPoint}.${remainingCentisThisPoint}`

const timeNext = `${durationNextPoint}.${remainingCentisNextPoint}`

const generateSubtitleLine = (offset: number, content: string): string => {

return `\nDialogue: 0,${timeThis},${timeNext},Default,,${videoWidth * offset},0,${videoHeight * 0.05},,${content}`

}

assFile = assFile.concat(generateSubtitleLine(0.1, subtitleDataString1))

assFile = assFile.concat(generateSubtitleLine(0.4, subtitleDataString2))

assFile = assFile.concat(generateSubtitleLine(0.7, subtitleDataString3))

})

Cool, but the generateSubtitleLine won't work, as the multiplier values are different for each subtitle line (0.1*videoWidth, 0.4*videoWidth, etc)

Cool, but the generateSubtitleLine won't work, as the multiplier values are different for each subtitle line (0.1*videoWidth, 0.4*videoWidth, etc)

That's an argument on the function

rafaellehmkuhl requested review from patrickelectric and joaoantoniocardoso February 16, 2024 21:53

rafaellehmkuhl mentioned this pull request Feb 19, 2024

Allow adjusting telemetry interval #746

Merged

rafaellehmkuhl requested a review from JoaoMario109 February 19, 2024 21:23

rafaellehmkuhl force-pushed the fix-telemetry-timing branch 2 times, most recently from d268f74 to 06ff8b8 Compare February 21, 2024 17:28

joaoantoniocardoso reviewed Feb 22, 2024

View reviewed changes

src/libs/sensors-logging.ts Outdated Show resolved Hide resolved

sensors-logging: Fix calculation of duration intervals to consider mi…

567bbc0

…lliseconds

rafaellehmkuhl force-pushed the fix-telemetry-timing branch from 06ff8b8 to 567bbc0 Compare February 22, 2024 16:01

rafaellehmkuhl requested a review from joaoantoniocardoso February 22, 2024 16:01

patrickelectric requested changes Feb 22, 2024

View reviewed changes

patrickelectric mentioned this pull request Feb 27, 2024

sensors-logging: Improve code that generates the subtitle dialogs #776

Open

patrickelectric approved these changes Feb 27, 2024

View reviewed changes

patrickelectric merged commit d2767fb into bluerobotics:master Feb 27, 2024
7 checks passed

rafaellehmkuhl deleted the fix-telemetry-timing branch March 7, 2024 14:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sensors-logging: Fix calculation of duration intervals to consider milliseconds #739

sensors-logging: Fix calculation of duration intervals to consider milliseconds #739

rafaellehmkuhl commented Feb 16, 2024

patrickelectric Feb 22, 2024

rafaellehmkuhl Feb 22, 2024

rafaellehmkuhl Feb 22, 2024

rafaellehmkuhl Feb 23, 2024

rafaellehmkuhl Feb 27, 2024

patrickelectric Feb 27, 2024

patrickelectric Feb 22, 2024

joaoantoniocardoso Feb 22, 2024

patrickelectric Feb 22, 2024

-      const timeThis = `${durationHoursThisPoint}:${durationMinutesThisPoint}:${durationSecondsThisPoint}.${remainingCentisThisPoint}`
-      const timeNext = `${durationHoursNextPoint}:${durationMinutesNextPoint}:${durationSecondsNextPoint}.${remainingCentisNextPoint}`
-      assFile = assFile.concat(`\nDialogue: 0,${timeThis},${timeNext},Default,,${0.1*videoWidth},0,${0.05*videoHeight},,${subtitleDataString1}`)
-      assFile = assFile.concat(`\nDialogue: 0,${timeThis},${timeNext},Default,,${0.4*videoWidth},0,${0.05*videoHeight},,${subtitleDataString2}`)
-      assFile = assFile.concat(`\nDialogue: 0,${timeThis},${timeNext},Default,,${0.7*videoWidth},0,${0.05*videoHeight},,${subtitleDataString3}`)
+      const timeThis = `${durationThisPoint}.${remainingCentisThisPoint}`
+      const timeNext = `${durationNextPoint}.${remainingCentisNextPoint}`
+      const generateSubtitleLine = (offset: number, content: string): string => {
+        return `\nDialogue: 0,${timeThis},${timeNext},Default,,${videoWidth * offset},0,${videoHeight * 0.05},,${content}`
+      }
+      assFile = assFile.concat(generateSubtitleLine(0.1, subtitleDataString1))
+      assFile = assFile.concat(generateSubtitleLine(0.4, subtitleDataString2))
+      assFile = assFile.concat(generateSubtitleLine(0.7, subtitleDataString3))
+    })

sensors-logging: Fix calculation of duration intervals to consider milliseconds #739

sensors-logging: Fix calculation of duration intervals to consider milliseconds #739

Conversation

rafaellehmkuhl commented Feb 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment