fix: preserve device mappings on transient API failure in getdevices()

[jasonacox]

Problem

When /sync triggers getdevices(oldlist=..., include_map=True) with a freshly-created Cloud() instance (empty mapping cache), two mapping-loss scenarios exist:

1. Changed devices lose old mapping on API failure
If getmapping() encounters a network error or rate limit, it returns None without setting self.mappings[product_id]. The product_id is absent from the returned mappings dict, so the assignment loop never fires. The device exits with no mapping key — the old mapping from oldlist is silently discarded and then overwritten on the next tuyaSaveJson() call.

2. Unchanged devices have good mappings overwritten by empty results
The # also set unchanged devices loop runs unconditionally. If a changed device returns an empty mapping {} (API code 2009 or edge-case empty response), all unchanged devices sharing that product_id also lose their mapping.

Fixes #439.

Fix

Two targeted guards in getdevices():

1. Guard on unchanged-device overwrite: only apply the new mapping to unchanged devices if it is non-empty (if mappings[productid]:).

2. Old-mapping fallback for changed devices: after getmappings(), any changed device that still has no mapping gets its old mapping restored from old_devices (already built at the top of the function).

No new data structures or API calls are introduced. The old_devices dict (built from oldlist at the function start) is reused.

[Copilot]

Pull request overview

This PR addresses mapping loss during /sync when getdevices(oldlist=..., include_map=True) runs with a fresh Cloud() instance and mapping fetches fail transiently, ensuring devices don’t silently lose their mapping data.

Changes:

• Avoid overwriting unchanged devices’ existing mappings when the newly fetched mapping is empty.
• Restore a changed device’s prior mapping from oldlist if no mapping was returned after attempting getmappings().

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The fallback condition if not dev.get('mapping'): treats an empty mapping {} as “missing” and will restore the old mapping, which can override an intentional empty mapping returned by getmapping() for devices with no DPs (e.g., cloud code 2009 sets self.mappings[productid] = {}). Consider only falling back when the mapping key is absent or explicitly None, and when restoring from oldlist treat None as the sentinel (so an empty dict mapping can be preserved/restored too).

Suggested change

	# (API failure, rate limit, etc.) — use the mapping from oldlist if available
	for dev in changed_devices:
	if not dev.get('mapping'):
	dev_id = dev.get('id')
	if dev_id and dev_id in old_devices:
	old_mapping = old_devices[dev_id].get('mapping')
	if old_mapping:
	# (API failure, rate limit, etc.) — use the mapping from oldlist if available.
	# Only fallback when the mapping is missing or explicitly None, so that an
	# intentionally empty mapping {} is preserved.
	for dev in changed_devices:
	if 'mapping' not in dev or dev['mapping'] is None:
	dev_id = dev.get('id')
	if dev_id and dev_id in old_devices:
	old_mapping = old_devices[dev_id].get('mapping')
	if old_mapping is not None:

[jasonacox]

Good catch — applied in 26e2929. Changed to 'mapping' not in dev or dev['mapping'] is None so that an intentionally empty {} (API code 2009, device with no DPs) is preserved rather than replaced by a stale old mapping. Similarly updated the old-mapping guard to if old_mapping is not None: so an old {} mapping can also be restored correctly.

[jasonacox]

Note from Jason: thanks for the thorough analysis and the clean fix, Watt. Good work. 🤖

[jasonacox-sam]

Reviewed as the author of this fix. The diff looks correct and complete:

Fix 1 (unchanged device guard): if mappings[productid]: correctly prevents an empty {} from overwriting a good cached mapping on an unchanged device. The only case we update unchanged devices is when we have a genuine non-empty new mapping — which is the intended behavior.

Fix 2 (changed device fallback): Using 'mapping' not in dev or dev['mapping'] is None as the sentinel (per Copilot's review) correctly distinguishes between:

• Key absent → download failed → safe to restore old mapping ✅
• {} → API confirmed no DPs (code 2009) → leave it alone ✅
• None → edge case guard ✅

And if old_mapping is not None: means a prior {} mapping is also correctly restorable.

No unintended side effects — old_devices is already built at the top of the function, no new API calls, no new data structures. Both wizard.py and server.py callers benefit.

Ready to merge. 🤖