Solve Android<=13 error 'Duration must be set with physical times'

[kelleyvanevert]

This fix should solve this issue: #174

However, I'm not a Kotlin coder, so I'm not 100% sure if this is correctly backwards compatible and working on Android 14 etc.

My aim was to make it backwards compatible by parsing the given datetimes with Instant first, and then converting them to LocalDateTimes. But I just asked Claude to write this Kotlin code for me, because it would take me considerably more time. It works on my Android 13 phone, but I haven't fully tested it yet on other versions or devices.

Alternatively, if we only need to accept ISO inputs, we can also just write it like so:

import java.time.LocalDateTime
import java.time.format.DateTimeFormatter

// ...

val formatter = DateTimeFormatter.ISO_DATE_TIME

val startTime =
  if (timeRangeFilter.hasKey("startTime")) 
    LocalDateTime.parse(timeRangeFilter.getString("startTime"), formatter) 
  else null

val endTime =
  if (timeRangeFilter.hasKey("endTime")) 
    LocalDateTime.parse(timeRangeFilter.getString("endTime"), formatter) 
  else null

Help checking this code is welcome :)

[matinzd]

Are there any other fallback strategies needed? Seems like AI commenting :D

[kelleyvanevert]

😅 because it is, as I mentioned above, it's Claude helping me write Kotlin which I usually don't.

[matinzd]

Ah I just saw it :) I need to make sure before I approve this PR. Hopefully this weekend I try to find some time.

[kelleyvanevert]

Yeah ofc take your time. Also, if the API only allows ISO anyway, it could be simplified like I described above, and we can remove the weird try catch stuff.

I've just patched it in my project for the moment, there's no rush or anything. Plus, we still need to check on Android 14 and such (I'll probably be doing so over the next few days).

[felipe-mapa]

Using ZoneId.systemDefault() changes current aggregateGroupByPeriod behaviour and grouped amount doesn't match Health Connect's value. aggregateGroupByPeriod should get combine amount from start of first day to end of last day. When adding a timezone, it gets from first day + timezone offset, instead of first day midnight.

For example if it's aggregated by 1 day, and you are in Japan (+9), it'll get aggregated value starting from 9am that day in user's device.

Found that using ZoneOffset.UTC can correct that. And get value matching Health Connect

Suggested change

	try {
	// First try to parse as Instant (handles formats with Z or offset)
	val instant = Instant.parse(dateTimeString)
	return LocalDateTime.ofInstant(instant, ZoneId.systemDefault())
	} catch (e: DateTimeParseException) {
	try {
	// Then try direct LocalDateTime parsing
	return LocalDateTime.parse(dateTimeString)
	} catch (e: DateTimeParseException) {
	try {
	// Then try parsing as ZonedDateTime
	return ZonedDateTime.parse(dateTimeString)
	.withZoneSameInstant(ZoneId.systemDefault())
	.toLocalDateTime()
	} catch (e: DateTimeParseException) {
	// Add more fallback parsing strategies if needed
	// If all else fails, throw an exception
	throw IllegalArgumentException("Unable to parse date-time: $dateTimeString", e)
	}
	}
	}
	try {
	// First try to parse as Instant (handles formats with Z or offset)
	val instant = Instant.parse(dateTimeString)
	return LocalDateTime.ofInstant(instant, ZoneOffset.UTC)
	} catch (e: DateTimeParseException) {
	try {
	// Then try direct LocalDateTime parsing
	return LocalDateTime.parse(dateTimeString)
	} catch (e: DateTimeParseException) {
	try {
	// Then try parsing as ZonedDateTime
	return ZonedDateTime.parse(dateTimeString)
	.withZoneSameInstant(ZoneOffset.UTC)
	.toLocalDateTime()
	} catch (e: DateTimeParseException) {
	// Add more fallback parsing strategies if needed
	// If all else fails, throw an exception
	throw IllegalArgumentException("Unable to parse date-time: $dateTimeString", e)
	}
	}
	}

[aleksamrakovic]

hey friends, do we have any estimation on this one 🙏

[antpuleo-rg]

Is anyone looking to merge this?

This is still an issue and makes aggregateGroupByPeriod completely unusable.

On android 16 I get an IllegalArgumentException with message Either use TimeRangeFilter with LocalDateTime or AggregateGroupByDurationRequest

[matinzd]

@antpuleo-rg Can you try using this PR as a patch and let me know if it works? I don't have time to test this PR that's why I haven't merged it yet.

[antpuleo-rg]

Hey @matinzd I've tested the patch in this issue:

#194

and it works as expect.

They are basically the same. I'm not sure if you need to catch then re-throw as per this PR though.

If you are worried about not getting the right start/end time you can take a look at this:

https://github.com/stridekick/react-native-health-connect/blob/main/android/src/main/java/com/healthconnectbridge/ActivityRecord.kt#L73

i.e.

// Start of the day (00:00:00)
val startTime = ZonedDateTime.of(localDate, LocalTime.MIN, ZoneId.systemDefault()).minusDays(days)

// End of the day (23:59:59)
val endTime = ZonedDateTime.of(localDate, LocalTime.MAX, ZoneId.systemDefault())

Although I have just been doing this on the JS side.