Skip to content

Fix ENCRYPTION_KEY var in README - #421

Open
LennyPenny wants to merge 1 commit into
sblibs:mainfrom
LennyPenny:patch-1
Open

Fix ENCRYPTION_KEY var in README#421
LennyPenny wants to merge 1 commit into
sblibs:mainfrom
LennyPenny:patch-1

Conversation

@LennyPenny

Copy link
Copy Markdown

Fixes var name being unabbreviated when defined.

Fixes var name being unabbreviated when defined.
@codecov

codecov Bot commented Nov 29, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bluetoothbot

Copy link
Copy Markdown
Collaborator

PR Review — Fix ENCRYPTION_KEY var in README

Correct instinct, wrong half of the fix — as written this re-breaks both lock examples with a NameError.

The underlying observation is real and well-spotted: the README's lock snippets did have a definition/usage mismatch, and naming the variable ENCRYPTION_KEY reads better alongside the Encryption key: output of scripts/get_encryption_key.py. Keeping the change docs-only and touching both snippets symmetrically is also the right shape.

  • Both hunks rename only the definition; the SwitchbotLock(..., KEY_ID, ENC_KEY, ...) call sites are untouched, so each snippet raises NameError: name 'ENC_KEY' is not defined.
  • Verified against upstream/main: the mismatch was already fixed there by aligning the call sites to ENC_KEY, so both examples are currently consistent and runnable — this PR would regress them.
  • The branch is based on a stale README.md (hunks at lines 31/55 vs. 67/91 on main, which now carries a "Troubleshooting key retrieval" section); it needs a rebase regardless.
  • Path forward: rebase and confirm — if the naming already reads consistently on main, this is resolved and closing is reasonable; otherwise rename the two call-site arguments as well so all four lines agree. Maintainer's call.

🔴 Blocking

1. Renaming only the definition reintroduces the NameError the examples already had
README.md:34

This renames the definition to ENCRYPTION_KEY, but the SwitchbotLock(...) call two lines below still passes ENC_KEY. Running the example verbatim now raises NameError: name 'ENC_KEY' is not defined.

Why it matters: this exact mismatch (in the opposite direction) was the original bug — the README used to define ENC_KEY and pass ENCRYPTION_KEY. It was fixed on main by aligning the call sites to ENC_KEY. I verified against upstream/main: both lock examples currently read ENC_KEY="XXXX…" and wolock[BLE_MAC].device, KEY_ID, ENC_KEY, model=LOCK_MODEL — consistent and runnable. Merging this definition-only rename would re-break both snippets for anyone copy-pasting them.

Also note this branch is based on an older README.md (the hunks land at lines 31/55; on current main those definitions sit at 67/91, after the "Troubleshooting key retrieval" section), so a rebase is needed before the diff is even meaningful.

How to fix — pick one:

  • Rebase on main and confirm the naming is already consistent; if so, this PR is resolved and can be closed.
  • If you prefer the unabbreviated name for readability, rebase and rename both the definition and the two SwitchbotLock(...) arguments in each snippet (4 lines total across the Unlock and Lock examples).
ENCRYPTION_KEY="XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" # The encryption-key with key-ID "XX"
2. Same definition/call-site mismatch in the Lock example
README.md:58

Identical problem in the second snippet: the definition becomes ENCRYPTION_KEY while lock.SwitchbotLock(wolock[BLE_MAC].device, KEY_ID, ENC_KEY, model=LOCK_MODEL).lock() still references ENC_KEY.

Why it matters: the Lock example is the one most users copy first; a NameError on the very first run is what drove the original support issue about the documented lock example not working.

Fix: same as above — either close as already-resolved on main, or rename the call-site argument in this snippet too.

ENCRYPTION_KEY="XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" # The encryption-key with key-ID "XX"

Checklist

  • Documented examples run as written — critical #1, critical #2
  • Change is consistent across all affected snippets — critical #1, critical #2
  • Branch is current with the base branch — critical #1
  • No hardcoded secrets (placeholders only)
  • Scope matches PR description (docs-only, no scope creep)

Automated review by Kōan (Claude) HEAD=06687e2 1 min 26s

@bluetoothbot bluetoothbot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Caution

Critical issues found.

  • Renaming only the definition reintroduces the NameError the examples already had
  • Same definition/call-site mismatch in the Lock example

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.

2 participants