First-class health: HealthKit, Health Connect, BLE sensors, workouts and simulator - #5475
Open
shai-almog wants to merge 77 commits into
Open
First-class health: HealthKit, Health Connect, BLE sensors, workouts and simulator#5475shai-almog wants to merge 77 commits into
shai-almog wants to merge 77 commits into
Conversation
First slice of the cross-platform health effort: the portable core that every port will implement against, plus the Bluetooth sensor layer, which needs no port code at all. com.codename1.health follows the Bluetooth API's shape -- a never-null facade with a no-op base class, reached via Display.getHealth(), with CodenameOneImplementation.getHealth() defaulting to null so no existing port breaks. What is here: - Vocabulary: HealthDataType (an interned value object rather than an enum, so constants can carry units without an initialisation-order hazard and forId() can stay total across versions), HealthUnit with affine conversion, and the HealthSample hierarchy covering quantity, category, series and session data. - HealthStore: public methods are final and do validation, unit normalisation, paging, bucket boundaries and the subscription registry; ports implement only the protected do* SPI. Anchors are persisted by the framework and stored only after a listener returns, so a crash costs one redelivered batch rather than the data. - com.codename1.health.sensors: built entirely on com.codename1.bluetooth.le, so it works on every port with BLE -- including desktop and JavaScript, where no health store exists. Byte-exact parsers for the standard SIG profiles. - com.codename1.health.workout: state machine, elapsed clock and statistics rollup in shared code, with RecordedWorkoutSession as the path for platforms with no live session -- which is what Google documents for Health Connect on phones, not a fallback. Two platform truths the API refuses to paper over: HealthKit cannot report read authorisation, so there is no hasReadPermission and the docs say to show "no data available" rather than "you denied access"; and Health Connect never wakes an app, so isPushDelivery() is a queryable fact. Aggregate results return null rather than a synthetic zero, since a day with no data and a day with no steps are different facts. Prerequisite refactors: SimulatorHookLoader gains a backward-compatible groups= form (one resource per classpath entry cannot otherwise host a second namespace), and the sim schedulers move to impl.javase.simulator so the health simulator can share them. Tests: 40 new, covering the fallback contract, unit conversion including the glucose factor, and the 0x2A37 traps -- unsigned reads, the contact-supported/detected pair, kilojoule energy, and variable-count RR intervals. Full suites green: 4213 core, 176 javase. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Desktop, JavaScript and the simulator have no HealthKit or Health Connect, but that does not have to mean a no-op. LocalHealthStore is a real store -- reads, writes, deletes and aggregates all work -- so aggregation logic, chart code and unit handling can be developed and unit-tested on a laptop rather than only on a device. It reports HealthAvailability.LOCAL_ONLY rather than pretending to be a platform store, because the difference matters: nothing else writes into it, so an app whose purpose is reading what a watch or another app recorded should say the feature needs a phone instead of showing an empty chart. Wired into the JavaSE, Windows, Linux and JavaScript ports. Workouts and the Bluetooth sensor layer already worked on all four and are unchanged. Being the only store with no platform behind it, this is also where the shared aggregation rules get exercised. The tests pin the two that are easiest to get wrong and hardest to notice: an empty bucket reports null rather than zero, and calendar-day buckets follow the calendar across the 2026-03-08 spring-forward transition, where a Los Angeles day is 23 hours and a fixed 86_400_000 bucket would quietly drift. 15 new tests; full core suite green at 4228. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit resolved a persisted background-listener class name with Class.forName, following GeofenceManager. That was the wrong precedent to copy: LocationManager needs it because it works around obfuscation with keep rules, and reusing the shape here inherited the fragility without the reason. Reflection is specifically bad on the platform this feature exists for. A class referenced only by a string is invisible to the iOS and JavaScript translators' dead-code elimination, so it can be stripped from the very build that needs it, and obfuscation renames it so a name persisted by an earlier version stops resolving. The old docs had to tell developers to "keep the class reachable" -- a warning that was really an admission the mechanism was unsound. The build server already scans bytecode for interface implementations and already injects registration code at startup, which is what it is for. So HealthBackgroundListenerFactory is a build-generated binding: the builder finds HealthBackgroundListener implementations, emits a factory that constructs each with a direct `new`, and registers it via HealthStore.setBackgroundListenerFactory. A real reference, which both dead-code elimination and obfuscation follow correctly. There is deliberately no reflective fallback. Where no bindings exist -- the simulator, unit tests -- delivery is skipped and the anchor is not advanced, so the changes are redelivered later rather than lost. Generating the factory is now part of the build-tooling work. 5 new tests. The two positive-delivery cases initially passed alone and failed in the full suite, which was a real ordering flake rather than a harness quirk; they now wait on a CountDownLatch through the established UITestBase.waitFor helper. Full suite run three times: 4233, no failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nutrition closes a gap: HealthDataType.NUTRITION existed and its docs referenced com.codename1.health.nutrition, which had not been written. A NutritionSample carries a sparse set of Nutrient amounts rather than the forty nullable fields both platforms model, and a nutrient that was never measured reads back as null rather than zero -- the same distinction aggregate buckets make. HealthManifestFragments emits the Health Connect manifest pieces: per-type permissions, the provider <queries> entry, and the permissions-rationale activity, which is not optional -- Health Connect silently declines to show its consent dialog to an app that lacks one. Permissions come from build hints because the type an app uses is a field reference and the class scanner's visitFieldInsn is an empty override, so the set genuinely cannot be inferred; that also matches Play policy on declaring exactly what you use. Duplicate suppression is quote-delimited, since READ_HEART_RATE is a prefix of READ_HEART_RATE_VARIABILITY, READ_EXERCISE of READ_EXERCISE_ROUTE, and READ_HEALTH_DATA of both its longer forms. HealthListenerBindings generates the background-listener factory promised when the reflection was removed: a direct `new` per listener, plus -keepnames so the persisted name key survives obfuscation. One hazard found while wiring AiDependencyTable and now pinned by tests: entries match with startsWith, so com/codename1/health/ also matches the sensors subpackage. Putting the androidx.health dependency there would give an app that only reads a heart-rate strap a Health Connect dependency and a Play health-permissions review it has no business needing. The dependency moves to AndroidGradleBuilder, gated on health usage outside sensors; the table keeps only what is safe for both cases. 44 builder tests, including a golden token list so a HealthDataType addition that is not mirrored into the builder table fails CI. Full plugin suite green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Detection is deliberately two-level. usesHealth means the app touched com.codename1.health at all; usesHealthStore means it touched something outside the sensors subpackage. Only the latter gates HealthKit, its entitlement, the privacy-string requirement, the Health Connect dependency and the per-type permissions -- so an app that only streams a heart-rate strap does ordinary BLE and acquires no health-data review on either store. iOS fails the build, loudly, when a health app declares no NSHealthShareUsageDescription or NSHealthUpdateUsageDescription, and again when it writes health data without the update string. That is the opposite of the camera and bluetooth entries, which default their copy, and it is deliberate: Apple reviews health purpose strings against what the app actually does, so a placeholder is what gets an app rejected rather than what keeps the build working. It also emits the HealthKit entitlement, which has no Bluetooth precedent since CoreBluetooth is not entitlement-gated. Android validates what it cannot infer: the data types, since a type is a field reference the scanner cannot see, and the privacy-policy URL, since Play requires one and the rationale screen must link to it. It raises minSdk to 26 and refuses targetSdk below 30, where <queries> is not emitted and the provider is invisible. Extending Executor.ClassScanner with implementsInterface is what makes the generated listener bindings possible: the visitor already had the implementing class name and was discarding it, reporting only the interface. The other three builders get a no-op, since they generate no callbacks. The manifest fragments are staged in fields rather than written where they are computed, because xQueries is reset and the <application> body is assembled well after the permission block runs. Full plugin suite green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SimulatedHealthStore layers scripted permissions and one-shot fault injection over the real local store, and its default is deliberately the awkward one. HealthKit does not disclose read authorization: a denied read returns an empty result, indistinguishable from having no data, so an app cannot infer what a user is hiding. A developer testing against a permissive store meets this for the first time in review or in production. So IOS_OPAQUE is the default policy, DENIED_SILENT reproduces the trap exactly -- authorization resolves true, the status reads UNKNOWN, queries come back empty with no error -- and GRANTED_BUT_NO_DATA produces observationally identical results, which is precisely why the API offers no hasReadPermission. Switching to ANDROID_EXPLICIT makes the same script fail loudly, as Health Connect does. Synthetic data rather than recorded fixtures. The Bluetooth simulator replays scrubbed traces, and that does not transfer: for BLE the sensitive parts are identifiers, so scrubbing leaves a trace tests can still assert on, whereas for health the sensitive part IS the asserted value. Scrubbing it either destroys the signal or commits quasi-identifying biometric data to a public repository's history forever. There is also no desktop HealthKit to record from. Everything is seeded, so exact-total assertions stay stable. The health menu is why SimulatorHookLoader gained the groups= form: a classpath entry carries one copy of simulator-hooks.properties, so a second namespace could not be a second file. A test parses the real shipped file rather than a fixture, which is what proves the extension actually works. 20 new tests; full javase suite green at 196. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A Health chapter in the developer guide, included after Bluetooth, with eight snippets that live in docs/demos and compile against the real API rather than being inline prose. The ordering is deliberate: permissions come before any read example, because the read-authorization asymmetry is the thing that will otherwise be discovered in App Review. The chapter says plainly that requestAuthorization resolving true means the user was asked rather than that anything was granted, that getReadAuthorizationStatus is UNKNOWN on iOS by design, and that a UI must therefore say "no data available" rather than accusing the user of denying access. The privacy section covers what the technical permissions do not: Apple's prohibition on advertising and iCloud storage and its specificity requirement for purpose strings, Google's health apps declaration form and the mandatory privacy-policy activity, and what Codename One itself does -- never uploads health data, never logs sample values, and fails the build rather than inventing a purpose string. Troubleshooting leads with "my query returns empty even though I granted permission", which is the support question this API will generate most. Verified locally rather than assumed: the snippets compile under JDK 17 via docs/demos process-classes, validate-guide-snippets passes across 636 include-backed blocks, and LanguageTool over the rendered guide reports zero matches. It caught two British spellings of mine, which are fixed in the prose rather than added to the accept list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Layered exactly like com.codename1.car, and for the same reason. androidx.health.connect is an AndroidX library whose API surface is Kotlin suspend functions, while the Codename One Android port compiles against a fixed old android.jar with no AndroidX, no Kotlin and no coroutines. The port cannot reference it -- and does not need to, because the port ships as source that the app's own Gradle compiles at a modern compileSdk, Kotlin sources included. So HealthConnectDelegate is a pure-Java seam speaking only Strings and primitives, CN1HealthConnectBridge.kt implements it and is copied into the generated project only for apps that use a health store, and AndroidHealthSupport publishes it at startup. Verified: the port still compiles under -Pcompile-android against the old android.jar, so nothing leaked across the boundary. Two deliberate choices worth recording. Aggregation is not delegated to Health Connect. The shared code already computes daylight-saving-correct bucket boundaries and a duration-weighted average, and having a second implementation on one platform is how the two come to disagree about a user's weekly total. The permissions-rationale activity lives in com.codename1.health rather than the impl package, because it is named from the manifest and needs a stable declared name -- the same reason the background-location activity sits outside impl. It is not optional: Health Connect silently declines to show its consent dialog to an app without one, so the failure mode is a permission request that never appears rather than one that is refused. HealthWire carries the line-delimited sample format shared with the iOS bridge. A year of heart rate is hundreds of thousands of samples and parsing that as JSON exhausts the ParparVM heap; unknown type ids and unit symbols skip the line rather than failing the page, so an older app reading a newer store loses the unfamiliar rows and keeps the rest. Core suite green at 4233. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CN1Health.m is gated on CN1_INCLUDE_HEALTH, which IPhoneBuilder uncomments only when the scanner saw health classes outside the sensors subpackage -- so a heart-rate-strap app ships without HealthKit symbols, without the entitlement, and without Apple's health-data review. The #else branch was written first and provides a no-op trampoline for every declared native, so a health-free app still links; the same branch is what compiles the feature out on tvOS and Mac Catalyst, where HealthKit does not exist. Two behaviours the native layer reports honestly rather than smoothing over. hkShareAuthorizationStatus covers write types only, and there is deliberately no read equivalent: HealthKit does not expose read authorization, so a read query is the only evidence there is. HKErrorDatabaseInaccessible maps to its own retryable error rather than being collapsed into "no data" -- the store is encrypted at rest and unreadable before first unlock, which is exactly when a background observer fires. IOSImplementation.getHealth() throws before constructing anything when no HealthKit privacy string is declared, following getLocationManager(). A missing usage description is a developer bug that gets an app rejected, so it must not be swallowable into an AsyncResource error nobody reads. Two gaps in the surrounding slices, now closed: HealthKit is weak-linked for tvOS, which would otherwise fail to link against the iOS slice's reference, and WatchNativeBuilder emits the health privacy strings and the workout-processing background mode. The watch Info.plist previously carried no privacy strings at all, which would have failed at runtime on the richest HealthKit target there is. Aggregation is deliberately left to shared code on both ports, so the bucket arithmetic has one implementation rather than two that can drift. Full plugin suite green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tier 1 gains the wire-format, sensor-parser, workout and nutrition suites. The parser cases each target a trap that produces plausible wrong numbers rather than an obvious failure: cycling power read unsigned turns a back-pedal into 65531 W, the weight scale uses a different resolution per unit system rather than a conversion applied afterwards, temperature uses the 32-bit IEEE-11073 FLOAT while blood pressure and glucose use the 16-bit SFLOAT, and a cuff or thermometer that fails sends a reserved value that surfaces as 2047 mmHg if it is let through. The wire-format cases pin the behaviour that matters for longevity: an unknown record type or unit skips its line rather than failing the page, so an older app reading a store a newer OS has extended loses the unfamiliar rows and keeps the rest. HealthConformanceTest runs on device on every port. It asserts only what must hold everywhere -- the facade and sub-facades are non-null, capability queries answer rather than throw, operations return a resource rather than hanging -- plus exact results for the pure functions, since unit conversion, the flags-byte parsing and the bucket boundaries have to agree across ParparVM, ART, the browser and the desktop JVM. Unsigned reads are called out specifically as the likeliest place for a translator to diverge. One defect found while reviewing: readSamples paged by mutating the caller's SampleQuery, leaving the last page token stuck on it, so a query object reused for a second read would silently resume mid-way through the first. It now pages over a copy. Suites: 4291 core, 196 javase, 322 plugin. Repo gates green -- since-tags, package-info, markdown javadoc, guide snippets, paragraph capitalization. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An actual iOS build found three ways the native bridge was wrong. It had never been compiled -- the ParparVM headers only exist after translation -- so this is exactly the class of defect the missing CI leg was hiding. Native definitions omitted the instance parameter. ParparVM emits `(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT __cn1ThisObject, ...)` for an instance native, and every hk* function declared only the thread state, so each argument was read one slot early. Arrays used an XMLVM type that does not exist in this VM. The String[] parameters of hkRequestAuthorization are JAVA_ARRAY, walked via a->length and (JAVA_ARRAY_OBJECT *)a->data, as cn1btUuidArray does. Callbacks used a macro that does not exist. Calling into Java from a GCD block attaches the thread with getThreadLocalData() rather than CN1_THREAD_STATE_MULTI_THREADED. Verified end to end rather than by inspection: hellocodenameone now references com.codename1.health from main sources, so the scanner fires and the builder does real work. The generated project confirms CN1_INCLUDE_HEALTH, INCLUDE_HEALTHSHARE_USAGE and INCLUDE_HEALTHUPDATE_USAGE all flipped and HealthKit.framework linked, and xcodebuild compiles CN1Health.m for arm64 against the iOS 26.2 SDK -- BUILD SUCCEEDED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…device builds A real Gradle build caught the generator emitting `new Outer$Inner()`. The dollar form is the binary name Class.getName() returns, which is correct as the lookup key, but it is not valid Java source for a constructor call -- and a listener nested inside the class that subscribes is the common case, so this would have failed for most apps that use background delivery. The key stays the binary name; the constructor call now uses the dotted source name. Regression test added. Both native layers are now verified by real builds rather than by inspection, which is what the earlier commits could not claim: iOS -- ParparVM translation plus xcodebuild for arm64 against the iOS 26.2 SDK. The generated project confirms CN1_INCLUDE_HEALTH and both INCLUDE_HEALTH*_USAGE defines flipped and HealthKit.framework linked. Android -- Gradle assembleDebug at compileSdk 36. The shipped dex contains CN1HealthConnectBridge, the generated CN1HealthListenerBindings, AndroidHealthSupport and androidx.health.connect.HealthConnectClient, so the Kotlin bridge genuinely compiled against Health Connect rather than being skipped. The merged manifest carries exactly the declared permission set -- READ_STEPS, READ_HEART_RATE, WRITE_STEPS -- plus the rationale activity, the API-34 ViewPermissionUsageActivity alias and the provider <queries> entry. hellocodenameone now references com.codename1.health from main sources and declares the required build hints, so these paths stay exercised. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six real findings across two modules, all in code this branch added. AndroidHealthSupport tripped EI_EXPOSE_STATIC_REP2 for storing a mutable object in a static field. That is the whole point of the registry, and AndroidCarSupport carries an exclusion for exactly the same pattern, so this mirrors it rather than contorting the design. Verified by removing the exclusion and watching the finding come back. The other five are genuine and are fixed rather than excluded: - Two anonymous callbacks in HealthStore and one in HealthSensors held a synthetic reference to their enclosing object. They are now built in static factory methods, matching how Bluetooth dispatches its EDT runnables and how the rest of this branch already did it. - validateForWrite iterated keySet and then looked each value up again, a second hash probe per entry. Now entrySet. - readPageInto evaluated the same limit comparison twice; hoisted, which also makes the trimming comment easier to follow. Worth recording for the next person: `spotbugs:check` reads a cached report, and core-unittests compiles the core sources itself, so a fix appears to have no effect until the module is recompiled. Re-running with test-compile is what actually re-analyses. Also caught while writing the exclusion: an em-dash written as `--` inside an XML comment is illegal and made the whole filter file unreadable, which silently dropped every existing exclusion rather than failing loudly. Suites still green: 4304 core, 196 javase. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
CI's developer-guide job runs Vale in addition to LanguageTool, which I had not run locally -- LanguageTool was clean, so I wrongly concluded the prose gates were satisfied. Vale reported 55 issues, all of them in the new chapter; every other chapter was already clean, so this was entirely mine. Most were the Microsoft style rules the guide follows: contractions, a few adverbs carrying no weight, punctuation inside quotes, and two phrases in first person. Fixed in the prose rather than allowlisted, since the rest of the guide observes the same rules. One case was not a prose problem. "Grant Write, Silently Deny Read" is the simulator's menu label, so the docs have to match the UI, and a vale-skip comment does not work inside a table row. The label is ours to choose, so it becomes "Grant Write, Deny Read Without Error" -- which names the behaviour that matters (the read fails with no error) rather than the manner, and is clearer for it. Renamed in the hooks properties, the test that pins it and the chapter together. Vale now reports zero across all 77 guide files, LanguageTool zero over the rendered guide, snippets and paragraph capitalization pass, javase suite green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Cloudflare Preview
|
Contributor
✅ ByteCodeTranslator Quality ReportTest & Coverage
Benchmark Results
Static Analysis
Generated automatically by the PR CI workflow. |
Collaborator
Author
|
Companion BuildDaemon PR: https://github.com/codenameone/BuildDaemon/pull/162 The daemon carries its own copies of |
The Ant build compiles core with -bootclasspath Ports/CLDC11/dist/CLDC11.jar, so only the CLDC profile's java.* is visible. Maven compiles against the JDK's rt.jar with -source 1.5 and never saw this, which is why every local check passed while the javase-simulator-tests job failed: the code would not have compiled for a device at all. Three APIs I used are not in that profile: - Calendar.getTimeInMillis() and setTimeInMillis(long) are protected there, so the instant has to move through getTime() / setTime(new Date(millis)). - Calendar.clear() and the six-argument set(y, m, d, h, mi, s) do not exist, so GattDateTime sets each field individually. - Calendar.setFirstDayOfWeek does not exist. The week bucketing already walked back to the configured first day by hand, so the call was redundant as well as unavailable. - java.lang.Math has no pow(). MathUtil.pow is the framework's own and is already what the rest of core uses, so the IEEE-11073 decoders use it. Worth knowing for anything else added to core: a green Maven build is not evidence that core compiles for a device. `ant core` is, and it needs `ant -f Ports/CLDC11 jar` first or it checks against a stale CLDC jar and reports unrelated failures in the calendar package. Verified: ant core BUILD SUCCESSFUL against a freshly built CLDC11.jar, and 4304 core tests still green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 12 screenshots: 12 matched. |
Collaborator
Author
|
Compared 148 screenshots: 148 matched. Benchmark Results
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 181 screenshots: 181 matched. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99df7e5b33
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ty guard setWriteToStore(true) is HealthKit use, and the sensors-package exemption hid it on iOS exactly as it did on Android -- so the build omitted the framework, the entitlement and the purpose-string check for a documented flow. The health-sensor dependency entry supplied only NSBluetoothAlwaysUsageDescription, which arrived in iOS 13. The ordinary Bluetooth entry beside it also carries NSBluetoothPeripheralUsageDescription, which iOS 12 checks before letting CoreBluetooth start, so a sensor-only app naming the health facade rather than com.codename1.bluetooth was terminated on the supported floor. Both were the same shape as two earlier defects: a classification fixed in the Android scanner and never mirrored to iOS, found each time by someone reading the other file. HealthScannerParityTest now fails the build when the two disagree about sensor write-through, the shared value types, or workouts being read and write. It matches source text, which is crude -- the rules live in anonymous visitor callbacks with no seam to call -- but crude and load-bearing beats absent, and it caught a real asymmetry on its first run. 355 plugin tests, 110 daemon. Twin of BuildDaemon d57551f. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…per type isBackgroundDeliverySupported() returned true whenever Health Connect was available, while the base contract is that changes arrive without the app asking. Nothing here does that -- there is no push and no lifecycle hook -- so an app could branch away from scheduling its own drain and receive nothing, which looks exactly like having no data. It answers false, as isPushDelivery() already did. The multi-type read limit is documented rather than silently exceeded. Neither static split works: dividing the budget cannot see a top-K that lives in one type, and the full budget per type cannot honour the cap, because the shared layer trims only when no continuation token remains and these types still have theirs. The fix is an incremental k-way merge across the types, which is a rewrite of that path; until it lands the cap is per type on Android, and setLimit, the bridge and the guide all say so and point at querying one type at a time where it has to be exact. 122 health tests, 355 plugin, 110 daemon. Twin of BuildDaemon 6b3a127. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1b90d787b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A scale reporting the Weight Scale Service's 0xFFFF "measurement unsuccessful" sentinel had it scaled into 327.675 kg, which the session published and could write to the store as a real body mass. The other medical parsers here already reject their failure values; this one now does too. A flattened series returned every measurement in the record, because the store matched the enclosing span -- so a one-minute query over an hour-long heart-rate record returned the hour. Each measurement is tested against the requested range. Flattening also broke the requested sort order: a port orders records, and a series carries its points in whatever order it was built with, so an expanded page followed neither direction. The expanded page is sorted by measurement. HealthSubscription.isPushDelivery() still told callers that iOS pushes and that Android drains at startup and foreground. Both are false, and this was the third copy of that claim; it says what actually happens and what the app has to do instead. Also mirrors the daemon's invokedynamic scanning: a method reference like store::readSamples was invisible to feature detection entirely. 150 health tests, 355 plugin, 110 daemon. Twin of BuildDaemon 8bc9465. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2679aecc91
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Narrowing the base entitlement to read/write/workout usage left the sub-capability blocks emitting from their hints alone, so an availability-only app setting ios.health.backgroundDelivery produced background-delivery with no com.apple.developer.healthkit under it. That is not a capability Apple can enable: it fails signing against a profile, and would not have worked if it had signed. The explicit hints count toward the base gate. 355 plugin tests, 110 daemon. Twin of BuildDaemon 5bd94b6. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 093a240d70
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
fireChanges returns whether it queued a delivery. A subscription restored after a relaunch with no live listener and no persisted background class has nothing to hand a batch to, and both ports counted it regardless -- so drainChanges reported batches nobody received, which reads to a caller as handled. Every counting site asks first. A sleep stage outside its session's span was copied in without validation, so a session could report more time asleep than it lasted and carry stage detail outside the very range the record is queried by. Both the factory and addStage reject it. 150 health tests, 355 plugin, 110 daemon. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d98213f9f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
NaN and the infinities were accepted as quantities and as series values, and the local store persisted them -- every later total, average, minimum and maximum inherited them, while a mobile store would have refused the same write. The simulator was the one place a malformed number could get in and look fine. Both constructors reject them. The cycling power parser dropped flags bit 0x0002, so a left-referenced pedal balance and an unqualified one produced indistinguishable objects while the getter told callers to consult a bit they could not see. The qualifier is retained and exposed. RR intervals are decoded and not delivered as samples, and now say why: they are the input to an HRV calculation rather than a measurement of one, and no platform has a type that would store them. An app that needs HRV subscribes to 0x2A37 through com.codename1.bluetooth.le and parses the raw value itself -- both public API, and composing those beats inventing a health type nothing can read back. 163 health tests, 110 daemon. Twin of BuildDaemon 92e0c07. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d117f9890
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…he iOS cursor - SleepSample: drop null stage entries instead of storing them, and count overlapping spans once. Platform sleep categories overlap, so summing them reported more time asleep than the session lasted. - SubscriptionRequest: reject control characters in the id. The registry that survives process death is newline- and tab-delimited and written unescaped, so such an id came back different -- or not at all. - WorkoutSample: reject energy and distance totals in the wrong dimension, at the setter, the way BloodPressureSample already does. - GlucoseMeasurement: treat 0x8000 as "time offset unknown" rather than -32768 minutes, which moved readings back 22.8 days. - BleSensorSession: a discovery or subscribe failure on the reconnect ladder is retried rather than retiring the session, bounded to three consecutive attempts. - IOSHealthStore: drain a tied instant before stepping over it, so a truncated page made entirely of records sharing the cursor timestamp no longer loses the ones that did not fit. - CN1HealthConnectBridge: report a whole series over the record's own interval, not the extent of its measurements. Every fix is pinned by a test verified to fail without it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6c8fff1a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+1746
to
+1748
| List<HealthChangeBatch> chunks = applyOptions(batch, sub); | ||
| synchronized (subscriptions) { | ||
| pendingDeliveries += chunks.size(); |
There was a problem hiding this comment.
When a platform page contains more additions than maxSamplesPerBatch, applyOptions() creates multiple HealthChangeBatch chunks and the listener is invoked once per chunk, but fireChanges() returns only one boolean and each port increments its delivered count only once. For example, 250 additions with a cap of 100 result in three callbacks while drainChanges() resolves with 1, contradicting its documented number-of-batches result; return or otherwise propagate the number of queued chunks.
Useful? React with 👍 / 👎.
Comment on lines
+358
to
+362
| if (reconnectFailures >= MAX_RECONNECT_FAILURES) { | ||
| setState(SensorSessionState.FAILED); | ||
| forgetFromManager(); | ||
| fireError(wrapped); | ||
| return; |
There was a problem hiding this comment.
When the third reconnect attempt reaches a discovery or subscription failure, the peripheral is already connected, but this terminal branch marks the session FAILED and returns before the disconnect() below. The session is then removed from the active registry and its reconnect listener no longer retries from FAILED, leaving an unusable BLE connection and listener alive until the caller explicitly stops a handle it was told had already ended; disconnect and remove the listener on this terminal path.
Useful? React with 👍 / 👎.
Comment on lines
+112
to
+114
| if ((flags & FLAG_RR_INTERVALS) != 0) { | ||
| // The count is not transmitted: whatever remains is RR data. | ||
| int count = r.remaining() / 2; |
There was a problem hiding this comment.
When the RR-present flag is set and the payload has an odd number of remaining bytes, integer division silently ignores the final byte and r.isValid() remains true because no out-of-bounds read occurred. The parser therefore accepts a truncated notification while dropping half of an RR interval, corrupting HRV input for callers using this public parser; require an even remainder before decoding the pairs.
Useful? React with 👍 / 👎.
Comment on lines
+87
to
+91
| /// - **iOS / watchOS** -- HealthKit, including background delivery and | ||
| /// (on watchOS, and on iOS 26 and later) live workout sessions. Requires | ||
| /// the HealthKit entitlement and privacy usage strings; the build fails | ||
| /// with an actionable message if they are missing. | ||
| /// - **Android** -- Health Connect, including exercise sessions. Requires |
There was a problem hiding this comment.
Fresh evidence after the earlier lifecycle-documentation fixes is that this top-level platform summary still advertises iOS background delivery/live sessions and Android exercise sessions, while both mobile stores report no background delivery and WorkoutManager explicitly reports both live-session capabilities as false everywhere in this release. A developer relying on the entry-point documentation can omit manual drains or design around an OS-owned workout that never exists, so describe the recorded/manual behavior implemented by this release.
Useful? React with 👍 / 👎.
Comment on lines
+80
to
+82
| /// Every method may be called from the EDT and returns immediately; every | ||
| /// callback is delivered on the EDT. The one qualification is | ||
| /// [HealthBackgroundListener], which may run with no visible UI after the |
There was a problem hiding this comment.
When a desktop, simulator, or JavaScript caller starts a store operation from a worker thread, LocalHealthStore.completeInline() runs its result chain on that same worker, contrary to this unconditional EDT guarantee. Application code following the primary API documentation may therefore update components directly from the callback and race the UI; qualify the guarantee as the developer guide already does for local-backed ports.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a first-class, cross-platform health API: HealthKit on iOS and watchOS, Health Connect on Android, live streaming from standard Bluetooth GATT health sensors, workout recording, and a scriptable simulator.
com.codename1.healthis greenfield — a repo-wide grep forhealthkit|health.connect|HKQuantity|androidx.healthreturned zero hits before this. Apps needing steps, heart rate, sleep, weight or workouts had no portable path and had to drop to per-platform native interfaces.Structurally this follows the Bluetooth API (#5399): a never-null facade over a no-op base class, reached through
Display.getHealth(), withCodenameOneImplementation.getHealth()defaulting tonullso no existing port breaks.What's here
HealthDataType/HealthUnit, four sample kinds,HealthStorewith afinal/SPI split, subscriptions with framework-owned anchorsRecordedWorkoutSession; sparseNutritionSampleLocalHealthStorefor desktop/JS/simulator, tvOS weak-link, watchOS plistHealthManifestFragments,HealthListenerBindings,AiDependencyTableentries, both buildersHealth.asciidoc+ 8 compiled snippetsDesign decisions worth reviewing
No reflection for background listeners. The first draft resolved a persisted listener class with
Class.forName, followingGeofenceManager. That was the wrong precedent to copy —LocationManagerneeds it to work around obfuscation, and reusing the shape inherited the fragility without the reason. A class referenced only by a string is invisible to the iOS and JavaScript translators' dead-code elimination, so it can be stripped from the very build that needs it. The build server now scans forHealthBackgroundListenerimplementations and generates a factory that constructs each with a directnew. This required extendingExecutor.ClassScannerwithimplementsInterface— the visitor already had the implementing class name and was discarding it.Strict privacy posture, unlike camera/bluetooth. The build fails when a health app declares no
NSHealthShareUsageDescription, rather than defaulting a placeholder. Apple reviews health purpose strings against what the app actually does, so a generic placeholder is what gets an app rejected — it would not even achieve the "keeps the build working" goal.Android permissions are hint-driven, not inferred. A data type is referenced as a constant, which compiles to a field read, and
Executor.visitFieldInsnis an empty override — so the permission set genuinely cannot be derived from bytecode. This also matches Play policy on declaring exactly what you use.The sensor subpackage is deliberately kept out of the health-data path.
AiDependencyTablematches withstartsWith, socom/codename1/health/also matchescom/codename1/health/sensors/. Putting theandroidx.healthdependency there would give an app that only reads a heart-rate strap a Health Connect dependency and a Play health-permissions review it has no business needing. The dependency lives inAndroidGradleBuilder, gated on health usage outside sensors; a test pins this.Aggregation is never delegated to the platform. Bucket boundaries and the duration-weighted average are computed once in shared code, so the two ports cannot drift apart on a user's weekly total.
Things the API refuses to claim
HealthKit deliberately does not disclose read authorization — a denied read is indistinguishable from having no data, so an app cannot infer what a user is hiding. Consequently there is no
hasReadPermission(),requestAuthorizationresolvingtruemeans the user was asked rather than that anything was granted, and the docs say to render "no data available" rather than accusing the user of denying access. The simulator defaults to reproducing this trap, since it is the behaviour real users produce and a permissive store never shows.Also: aggregate buckets return
nullrather than a synthetic0(a day with no data and a day with no steps are different facts);HealthSubscription.isPushDelivery()isfalseon Android because Health Connect never wakes an app; andWorkoutManager.isLiveSessionSupported()andisSensorCollectionSupported()are separate queryable facts.Verification
Both native layers were built with the real toolchains rather than checked by inspection — which found three defects inspection had missed, including
CN1Health.momitting the ParparVM instance parameter so every native argument was read one slot early, and the bindings generator emittingnew Outer$Inner(), which is not valid Java source and would have broken the common nested-listener case.xcodebuildarm64 against the iOS 26.2 SDK: BUILD SUCCEEDED. The generated project confirmsCN1_INCLUDE_HEALTHand bothINCLUDE_HEALTH*_USAGEdefines flipped andHealthKit.frameworklinked.#elsetrampolines link, which is the tvOS and Mac Catalyst path.assembleDebugat compileSdk 36: BUILD SUCCESSFUL. The shipped dex containsCN1HealthConnectBridge, the generatedCN1HealthListenerBindings,AndroidHealthSupportandandroidx.health.connect.HealthConnectClient, so the Kotlin bridge genuinely compiled rather than being skipped. The merged manifest carries exactly the declared permission set plus the rationale activity, the API-34 alias and the provider<queries>.hellocodenameonenow references the API from main sources and declares the build hints, so these paths stay exercised.Not verified
tvOS and watchOS platforms are not installed in the Xcode used, so those target builds — and therefore the
TV_OPTIONAL_FRAMEWORKSweak-link andWKBackgroundModesemission — are untested end to end. The compile-out path was verified by flipping the define, which exercises the same branch.No runtime execution. Everything is compile-and-link; nothing ran on a device, so the HealthKit query paths and the coroutine bridge are unproven at runtime. Dedicated
health-android.yml/health-ios.ymlCI legs are the natural follow-up, as neither bridge is compiled by any existing job.Coverage inside the bridges is partial by design: iOS implements availability, authorization, sample read and write; Android covers steps, heart rate and weight. Background delivery (
HKObserverQueryplus its completion watchdog), liveHKWorkoutSessionand glucose RACP are designed and documented but not implemented — the API reports them honestly rather than pretending.🤖 Generated with Claude Code