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

Mobovi TicWatch schemas #359

Merged
merged 5 commits into from
Apr 30, 2024
Merged

Mobovi TicWatch schemas #359

merged 5 commits into from
Apr 30, 2024

Conversation

danjjl
Copy link
Contributor

@danjjl danjjl commented Nov 6, 2023

Mobivi TicWatch Schemas to record:

  • PPG (2 channels)
  • ACC (3 channels)
  • GYR (3 channels)

Additionally information :

  • battery level
  • server NTP time status
  • device version

@danjjl
Copy link
Contributor Author

danjjl commented Nov 7, 2023

I specified the UTC timestamps and doubles in ms.
It seems most other schemas use UTC timestamps as doubles in s.
Does this mean the integer part of the double is a UTC in seconds with ms after the decimal point?

@Bdegraaf1234
Copy link
Member

Hi @danjjl

Sorry for the wait, in the future you can request a review from me or another contributor to get the ball rolling.

@Bdegraaf1234 Bdegraaf1234 self-requested a review November 20, 2023 16:52
Copy link
Member

@Bdegraaf1234 Bdegraaf1234 left a comment

Choose a reason for hiding this comment

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

My first remark would be that we should merge this into dev rather than master.

Second, the code does not compile on the ci server. Please make sure that the command ./gradlew assemble and ./gradlew check both succeed before we can merge these changes.

Lastly, testing the schemas locally would be appreciated.

@danjjl danjjl force-pushed the master branch 2 times, most recently from ebadfcb to fda7379 Compare March 5, 2024 12:27
@danjjl
Copy link
Contributor Author

danjjl commented Mar 7, 2024

@Bdegraaf1234 close to having solved all errors. Any idea what should be done to solve this last one?

Still getting one which I have difficulty understanding:

org.opentest4j.AssertionFailedError: expected: <> but was: <Validation FAILED:
Failed to load configuration /home/dan/ownCloud/epilepsy/RADAR-Schemas/specifications/passive/ticwatch-1.0.0.yml


>
	at app//org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at app//org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at app//org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
	at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
	at app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1141)
	at app//org.radarbase.schema.validation.SpecificationsValidatorTest$passiveIsYml$1.invokeSuspend(SpecificationsValidatorTest.kt:52)
	at app//kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at app//kotlinx.coroutines.internal.ScopeCoroutine.afterResume(Scopes.kt:32)
	at app//kotlinx.coroutines.AbstractCoroutine.resumeWith(AbstractCoroutine.kt:102)
	at app//kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:46)
	at app//kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:108)
	at app//kotlinx.coroutines.EventLoopImplBase.processNextEvent(EventLoop.common.kt:280)
	at app//kotlinx.coroutines.BlockingCoroutine.joinBlocking(Builders.kt:85)
	at app//kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking(Builders.kt:59)
	at app//kotlinx.coroutines.BuildersKt.runBlocking(Unknown Source)
	at app//kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking$default(Builders.kt:38)
	at app//kotlinx.coroutines.BuildersKt.runBlocking$default(Unknown Source)
	at app//org.radarbase.schema.validation.SpecificationsValidatorTest.passiveIsYml(SpecificationsValidatorTest.kt:49)
	at java.base@17.0.10/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base@17.0.10/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base@17.0.10/java.util.ArrayList.forEach(ArrayList.java:1511)

@Bdegraaf1234 Bdegraaf1234 changed the base branch from master to dev April 3, 2024 09:21
@Bdegraaf1234
Copy link
Member

Hey @danjjl, it looks like you did not define your units.

The units are defined here

You can add some ( I think PPG does not exist, maybe make it a non-abbreviated version) and use the existing definitions where possible (G, RADIAN_PER_SEC are the first ones I saw)

@danjjl
Copy link
Contributor Author

danjjl commented Apr 29, 2024

@Bdegraaf1234 I corrected units

@Bdegraaf1234
Copy link
Member

@danjjl The tests are not running because of merge conflicts, can you resolve those?

@danjjl
Copy link
Contributor Author

danjjl commented Apr 29, 2024

@Bdegraaf1234 should be good now

@Bdegraaf1234
Copy link
Member

Hi Jonathan, it seems the tests are failing again, are they passing when you run ./gradlew check?

@danjjl
Copy link
Contributor Author

danjjl commented Apr 30, 2024

@Bdegraaf1234 applied a fix.
./gradlew check now runs locally

Copy link
Member

@Bdegraaf1234 Bdegraaf1234 left a comment

Choose a reason for hiding this comment

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

Great!

"namespace": "org.radarcns.passive.ticWatch",
"type": "record",
"name": "TicWatchPPG",
"doc": "PPG collected using a Mobovi TicWatch.",
Copy link
Member

Choose a reason for hiding this comment

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

Please be a bit more descriptive in the documentation, what is PPG?

"name": "TicWatchGyroscope ",
"doc": "Data from 3-axis gyroscope sensor of a Mobovi TicWatch device.",
"fields": [
{ "name": "time", "type": "double", "doc": "Device timestamp in UTC (ms)." },
Copy link
Member

Choose a reason for hiding this comment

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

your question about time should probably be answered by the ticwatch docs

- type: ACCELEROMETER
sample_rate:
dynamic: true
unit: g
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 this should be G maybe? the tests are complaining about this unit.

image

- type: GYROSCOPE
sample_rate:
dynamic: true
unit: rad/s
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 this should be RADIAN_PER_SEC? the tests are not complaining here yet but that will probably be the next error

@Bdegraaf1234 Bdegraaf1234 merged commit e4415c2 into RADAR-base:dev Apr 30, 2024
3 checks passed
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