Skip to content

chore: remove rendundant xTaskCreate#1264

Merged
jonasdiemer merged 1 commit intocrosspoint-reader:masterfrom
ngxson:xsn/redundant_xtaskcreate
Mar 2, 2026
Merged

chore: remove rendundant xTaskCreate#1264
jonasdiemer merged 1 commit intocrosspoint-reader:masterfrom
ngxson:xsn/redundant_xtaskcreate

Conversation

@ngxson
Copy link
Contributor

@ngxson ngxson commented Mar 1, 2026 •
edited
Loading

Summary

Ref discussion: #1222 (comment)

Important note that this is a bug-for-bug fix. In reality, this branch WiFi.status() == WL_CONNECTED is pretty much a dead code because the entry point of these 2 activities don't use wifi.

It is better to refactor the management of network though, but it's better to be a dedicated PR.

AI Usage

While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it
helps set the right context for reviewers.

Did you use AI tools to help write this code? NO

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 1, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7dc5186 and 12b47a9.

📒 Files selected for processing (2)
  • src/activities/reader/KOReaderSyncActivity.cpp
  • src/activities/settings/KOReaderAuthActivity.cpp
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: cppcheck
  • GitHub Check: build
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-02-23T06:18:08.408Z 
Learnt from: raygan
Repo: crosspoint-reader/crosspoint-reader PR: 1116
File: src/activities/settings/WallabagSettingsActivity.cpp:57-75
Timestamp: 2026-02-23T06:18:08.408Z
Learning: When navigating from a settings activity to KeyboardEntryActivity (e.g., KOReaderSettingsActivity, WallabagSettingsActivity), call exitActivity() on the parent before calling enterNewActivity(new KeyboardEntryActivity(...)). The KeyboardEntryActivity should then call exitActivity() via its callbacks to return to the parent. Note: ConfirmationActivity does not require exitActivity() before entering KeyboardEntryActivity. Apply this pattern to similar settings activities in this module.

Applied to files:

  • src/activities/settings/KOReaderAuthActivity.cpp
📚 Learning: 2026-02-27T22:49:59.600Z 
Learnt from: ngxson
Repo: crosspoint-reader/crosspoint-reader PR: 1218
File: src/activities/ActivityManager.cpp:254-265
Timestamp: 2026-02-27T22:49:59.600Z
Learning: In this codebase, assertions are always enabled (no NDEBUG). Use assert() to crash on programmer errors and surface logic bugs during development and in production builds. Do not rely on asserts for runtime error handling; they should enforce invariants that must always hold. Keep asserts side-effect free and inexpensive, and avoid relying on them for user-visible failures. Include <cassert> where appropriate and document the invariant being tested.

Applied to files:

  • src/activities/settings/KOReaderAuthActivity.cpp
  • src/activities/reader/KOReaderSyncActivity.cpp
🧬 Code graph analysis (2)
src/activities/settings/KOReaderAuthActivity.cpp (5)
src/activities/reader/KOReaderSyncActivity.cpp (2)
  • onWifiSelectionComplete (53-82)
  • onWifiSelectionComplete (53-53)
src/activities/browser/OpdsBookBrowserActivity.cpp (2)
  • onWifiSelectionComplete (374-391)
  • onWifiSelectionComplete (374-374)
src/activities/network/CalibreConnectActivity.cpp (2)
  • onWifiSelectionComplete (64-71)
  • onWifiSelectionComplete (64-64)
src/activities/settings/OtaUpdateActivity.cpp (2)
  • onWifiSelectionComplete (13-51)
  • onWifiSelectionComplete (13-13)
src/activities/network/CrossPointWebServerActivity.cpp (2)
  • onWifiSelectionComplete (155-182)
  • onWifiSelectionComplete (155-155)
src/activities/reader/KOReaderSyncActivity.cpp (5)
src/activities/settings/KOReaderAuthActivity.cpp (2)
  • onWifiSelectionComplete (14-33)
  • onWifiSelectionComplete (14-14)
src/activities/browser/OpdsBookBrowserActivity.cpp (2)
  • onWifiSelectionComplete (374-391)
  • onWifiSelectionComplete (374-374)
src/activities/network/CalibreConnectActivity.cpp (2)
  • onWifiSelectionComplete (64-71)
  • onWifiSelectionComplete (64-64)
src/activities/settings/OtaUpdateActivity.cpp (2)
  • onWifiSelectionComplete (13-51)
  • onWifiSelectionComplete (13-13)
src/activities/network/CrossPointWebServerActivity.cpp (2)
  • onWifiSelectionComplete (155-182)
  • onWifiSelectionComplete (155-155)
🔇 Additional comments (2)
src/activities/reader/KOReaderSyncActivity.cpp (1)

205-209: LGTM! Clean consolidation of the WiFi-connected path.

Delegating to onWifiSelectionComplete(true) when WiFi is already connected eliminates code duplication and ensures the sync initiation follows the same flow (logging, state update, NTP sync, performSync()) regardless of how WiFi connectivity was established. This aligns with the established pattern in other activities like OtaUpdateActivity and OpdsBookBrowserActivity.

src/activities/settings/KOReaderAuthActivity.cpp (1)

54-58: LGTM! Consistent with the KOReaderSyncActivity refactoring.

Routing the already-connected path through onWifiSelectionComplete(true) ensures authentication follows the same flow (state = AUTHENTICATING, status update, performAuthentication()) whether WiFi was pre-connected or selected via the subactivity. This removes the redundant task creation and aligns both activities with a unified pattern.

📝 Walkthrough

Walkthrough

This PR consolidates WiFi-connected sync and authentication initiation logic by delegating to existing onWifiSelectionComplete(true) handlers. Both affected files replace inline state management and task spawning with calls to the unified completion handler, reducing code duplication across the two activities.

Changes

Cohort / File(s) Summary
Sync Initialization Consolidation
src/activities/reader/KOReaderSyncActivity.cpp		 Refactored WiFi-connected sync path to delegate to onWifiSelectionComplete(true) instead of manually setting SYNCING state, posting status, and spawning SyncTask; centralizes sync initiation logic.
Authentication Initialization Consolidation
src/activities/settings/KOReaderAuthActivity.cpp		 Refactored WiFi-connected authentication path to delegate to onWifiSelectionComplete(true), removing manual WiFi mode setup and task creation; reuses unified completion handler.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • daveallie
  • znelson
  • osteotek
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'chore: remove redundant xTaskCreate' accurately describes the main change—removing redundant xTaskCreate calls from sync and authentication flows.
Description check ✅ Passed The description is directly related to the changeset, explaining the bug-for-bug fix, referencing the original discussion, and acknowledging the dead code condition.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jonasdiemer jonasdiemer merged commit 307a660 into crosspoint-reader:master Mar 2, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@znelson znelson znelson approved these changes

@jonasdiemer jonasdiemer jonasdiemer approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ngxson @znelson @jonasdiemer