Skip to content

Conversation

@ovitrif
Copy link
Collaborator

@ovitrif ovitrif commented Oct 30, 2025

Description

This PR adds the ldk-node logs to the android logcat and refactors to centralize the logging concerns and unify the log message format.

The logging format was adopted from ldk-node (source) with some small tweaks so it plays nice with both android log levels and the ldk-node ones.

note to reviewer : please merge only AFTER #449 .

Preview

Android Studio 2025-10-30 000984

QA Notes

Run in emulator, check logcat, expect ldk-node logs lines alongside the app logs in chronological order.

@ovitrif ovitrif changed the base branch from master to feat/rgs-regtest October 30, 2025 11:34
@ovitrif ovitrif requested a review from jvsena42 October 30, 2025 11:35
@ovitrif ovitrif self-assigned this Oct 30, 2025
@jvsena42 jvsena42 requested a review from Copilot October 30, 2025 12:11
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements streaming of ldk-node logs to Android logcat while centralizing and unifying the logging architecture. The changes create a consistent log format that adopts ldk-node's logging pattern while making it compatible with Android log levels.

  • Refactors the Logger from an object to a more flexible class-based architecture with dependency injection
  • Implements LdkLogWriter to bridge ldk-node logs to Android logcat with proper log level mapping
  • Centralizes log formatting and file management for both Bitkit and LDK logs

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
Logger.kt Complete refactor from object to class-based architecture with unified log formatting and LdkLogWriter implementation
LightningService.kt Updates to use new LdkLogWriter instead of filesystem logging and improves error handling patterns
LogsRepo.kt Moves LogSource enum to Logger.kt for centralized logging concerns
DateTime.kt Adds utility function for UTC date formatters and LOG_LINE pattern for consistent timestamp formatting
Env.kt Reorganizes ldkLogLevel declaration and fixes initialization order issue
ServiceQueue.kt Adds LOG queue for dedicated logging thread management

@ovitrif
Copy link
Collaborator Author

ovitrif commented Oct 30, 2025

Really like the copilot summaries, though I wonder if it's able to generate them so nicely described if we no longer spend time to manually write relevant summary in the PR description. Worth a try though, it could save some valuable time... who knows 🤷🏻‍♂️.

@jvsena42
Copy link
Member

Really like the copilot summaries, though I wonder if it's able to generate them so nicely described if we no longer spend time to manually write relevant summary in the PR description. Worth a try though, it could save some valuable time... who knows 🤷🏻‍♂️.

I guess it is done like this in blocktank repo

@ovitrif ovitrif force-pushed the feat/logcat-ldk-logs branch from 258c574 to d6e639c Compare October 30, 2025 13:29
Base automatically changed from feat/rgs-regtest to master October 30, 2025 13:40
@ovitrif ovitrif removed the request for review from jvsena42 October 30, 2025 14:12
@ovitrif ovitrif force-pushed the feat/logcat-ldk-logs branch from d6e639c to b7d2941 Compare October 30, 2025 14:44
@ovitrif ovitrif requested a review from jvsena42 October 30, 2025 14:44
@ovitrif ovitrif enabled auto-merge October 30, 2025 14:44
@ovitrif
Copy link
Collaborator Author

ovitrif commented Oct 30, 2025

@jvsena42 This PR should be ready for review now 🙏🏻

Copy link
Member

@jvsena42 jvsena42 left a comment

Choose a reason for hiding this comment

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

Tested emulator -> check app and LDK Logs

@ovitrif ovitrif merged commit 1ee92e7 into master Oct 30, 2025
11 checks passed
@ovitrif ovitrif deleted the feat/logcat-ldk-logs branch October 30, 2025 16:36
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