Skip to content

docs(#7): ES8311 upstream readiness + engagement plan (hold per ADR 0004)#16

Open
junnncct1106 wants to merge 5 commits into
thc1006:mainfrom
junnncct1106:chore/es8311-upstream-readiness
Open

docs(#7): ES8311 upstream readiness + engagement plan (hold per ADR 0004)#16
junnncct1106 wants to merge 5 commits into
thc1006:mainfrom
junnncct1106:chore/es8311-upstream-readiness

Conversation

@junnncct1106

@junnncct1106 junnncct1106 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

What

Sets up issue #7 (upstream the ES8311 codec) for a fast, clean submission when
the upstream gate opens
— without opening a competing PR now. Per ADR 0004
the decision is engage, don't compete: there is already a live upstream ES8311
effort (zephyrproject-rtos/zephyr#107660), so we hold and prepare rather than
fork the effort.

This PR is planning/readiness only — it does not open the upstream PR. Issue #7 stays open until the gate lifts and the upstream contribution is actually made.

Changes

  • docs/issues/0007-es8311-upstream-readiness.md — the "ready to fire"
    checklist: live gate status, readiness checks (all green), the submission-time
    TODO (genericize board refs → split PR → compliance → coordinate #107660).
  • scripts/check_es8311_upstream_gate.shgh-based monitor that prints the
    live state of the gating PRs (#110205, #107655, #107660) and a
    GATE OPEN / STILL HOLDING verdict, so we don't have to track it by hand.
  • docs/07_UPSTREAM_PLAN.md (item 5) — reconciled with ADR 0004: drops the
    stale "open our own PR / ztest 9/9" framing for the engage-not-compete decision
    (ztest 11/11, capture/ADC route as the differentiator, hold conditions).
  • docs/drafts/es8311-upstream-comments.md — parked, ready-to-post text:
    Draft A (issue-Upstream the ES8311 audio codec driver (everest,es8311) to Zephyr #7 framing note) and Draft B (trimmed upstream engagement comment
    — mute-reg 0x31 confirmation + a real-StickS3 hardware-check offer). Not posted.

Asset state (re-verified 2026-06-11)

  • twister native_sim → 11/11 pass (tests/drivers/audio/es8311).
  • checkpatch under Zephyr's .checkpatch.conf0 errors / 0 warnings on the
    driver, emul, binding, Kconfig and test.
  • Driver is board-independent; the capture/ADC route is the value-add over
    #107660 (which disables ADC).

Gate status (still HOLDING)

Upstream PR State Gates us?
zephyr#110205 (our StickS3 board) OPEN, approved, not merged merge → clean board path
zephyr#107655 (base ESP32-S3-BOX-3) OPEN, active, not merged ADR gate: ES8311 resumes after this
zephyr#107660 (existing ES8311) OPEN, stalled since 6/5 the effort to coordinate with

Neither gating board PR has merged and no live successor ES8311 PR exists →
hold conditions still in force (ADR 0004). When the gate opens, run the
monitor script and follow the readiness checklist.

Refs #7.

Copilot AI review requested due to automatic review settings June 10, 2026 21:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds an ES8311 upstream “gate” tracker plus supporting documentation to help coordinate contributions with existing Zephyr upstream PRs, per ADR 0004.

Changes:

  • Added a gh-based script to report the live status of gating Zephyr PRs and emit a “GATE OPEN / STILL HOLDING” verdict.
  • Added Issue #7 readiness checklist and parked upstream comment drafts to streamline submission once gating conditions are met.
  • Updated the upstream plan to reflect ADR 0004’s “engage, don’t duplicate” approach for ES8311.

Reviewed changes

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

File Description
scripts/check_es8311_upstream_gate.sh New script to query gating PRs via GitHub CLI and print a verdict.
docs/issues/0007-es8311-upstream-readiness.md New checklist + gate-status snapshot for ES8311 upstream readiness.
docs/drafts/es8311-upstream-comments.md New “parked drafts” text for upstream engagement comments, gated by conditions.
docs/07_UPSTREAM_PLAN.md Updates plan section #5 to match ADR 0004 and current upstream coordination strategy.

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

Comment thread scripts/check_es8311_upstream_gate.sh Outdated
Z=zephyrproject-rtos/zephyr
gate_open=0

field() { gh pr view "$1" --repo "$Z" --json "$2" -q ".$2" 2>/dev/null || echo "?"; }
Comment thread scripts/check_es8311_upstream_gate.sh Outdated
Comment on lines +29 to +31
if { [ "$pr" = "110205" ] || [ "$pr" = "107655" ]; } && [ "$merged" != "" ] && [ "$merged" != "no" ] && [ "$merged" != "null" ]; then
gate_open=1
fi
Comment on lines +10 to +14
| Upstream PR | What | State | Gates us? |
|---|---|---|---|
| zephyr#110205 | `boards: m5stack: add M5Stack StickS3` (our maintainer thc1006) | **OPEN, APPROVED**, not merged | once merged → clean board-driven path for our codec |
| zephyr#107655 | `boards: espressif: ESP32-S3-BOX-3` (base board) | OPEN, CHANGES_REQUESTED, active (upd 6/10) | ADR gate: ES8311 work resumes after this lands |
| zephyr#107660 | `drivers: audio: ES8311 + BOX-3 speaker sample` (nnSiD) | OPEN, CHANGES_REQUESTED, **stalled since 6/5**, playback-only (ADC disabled) | the existing effort to coordinate with |

@thc1006 thc1006 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I went through this against the live upstream tracker and our own ADR 0004, and the facts hold up well. The three gating PR states are accurate as of today (110205 approved and not merged, 107655 changes-requested and active, 107660 changes-requested and last touched 6/5), the "no live successor PR" call checks out (a gh search only turns up 107660 and our own board PR), and the 11/11, capture-route, and mute-register claims all match what is actually in the tree. I spot-checked Draft B's mute line against drivers/audio/es8311.c: reg 0x31, bits 6:5, mask 0x9F set 0x60 is exactly what the driver does, so that one is safe to post when the time comes.

Two things I would fix before this merges:

  1. It still auto-closes #7. The body says "Refs #7" and "does not close #7", which is the intent I would expect, but GitHub still lists #7 under the PR's closing references (closingIssuesReferences returns #7), so merging as-is will close the issue anyway. Probably a leftover from an earlier "Closes #7" body or a manual link in the Development sidebar. Worth clearing that link so #7 stays open, since the whole point here is that the upstream work is not done yet.

  2. The gate script's verdict is looser than the ADR it enforces. check_es8311_upstream_gate.sh flips to GATE OPEN when 110205 OR 107655 merges, but ADR 0004's gate is 107655 (the base board) landing AND the ES8311 work coming back on a live PR. As written, our own board PR (110205) merging on its own would print "GATE OPEN, proceed to upstream", which is the opposite of the hold we are trying to hold. I would gate on 107655 merged plus a live successor PR existing, and drop 110205 from the trigger.

On Copilot's notes: the field() one is real. A gh failure returns "?", which the merge check counts as merged, so it stacks with point 2 and biases the script toward a false GATE OPEN. Treat "?" or any non-timestamp as not-merged. The malformed-table note I could not reproduce, the gate-status table reads as single-pipe valid GFM on my end.

None of this blocks the docs themselves. I would just sort the gate logic before merge since it is the one piece that could push us into a premature upstream action later.

@thc1006

thc1006 commented Jun 11, 2026

Copy link
Copy Markdown
Owner

@junnncct1106 u can request copilot for review
and fix only important problem.

those typo or document problem plz self review by cladue don't waste copilot credit

@thc1006

thc1006 commented Jun 11, 2026

Copy link
Copy Markdown
Owner

@junnncct1106 and always to run CI in local

@junnncct1106

Copy link
Copy Markdown
Contributor Author

Thanks for the careful review — addressed all three.

1. #7 auto-close — removed the issue link, so merging won't close #7. It stays open until the upstream work actually lands.

2. Gate too loosecheck_es8311_upstream_gate.sh now gates on #107655 merged AND a live successor PR, and I dropped #110205 from the trigger. Our own board PR merging alone no longer opens the gate.

3. field() returning "?" — it now returns empty on any gh error, and the merge check requires state == MERGED plus a real ISO timestamp. Verified offline that both a gh outage and a #110205-only merge stay STILL HOLDING.

The malformed-table note looks like a false positive on my end too — the gate table reads as valid GFM.

Also ran CI locally (verify.sh) — green.

junnncct1106 added a commit to junnncct1106/zephyr-m5stack-sticks3 that referenced this pull request Jun 11, 2026
Covers the four cases from the PR thc1006#16 review with a mock gh (no network,
no auth): base #107655 merged + a live successor -> GATE OPEN; base only,
our-board-#110205-only, and gh-down -> all STILL HOLDING. Guards against the
AND-gate and the false-merge ("?") regressions from reverting.

Signed-off-by: Hsiu-Chi Tsai <hctsai@linux.com>
Captures the verified-ready state (checkpatch clean, twister 11/11, DCO
signed, no AI footers, board-independent code, capture route as the
differentiator vs zephyr#107660) and the submission-time TODO (genericize
board/project references, split into a focused PR, compliance run,
coordinate per ADR 0004). Records the live upstream gate status as of
2026-06-11: #110205 approved-not-merged, #107655 active-not-merged, #107660
stalled — hold conditions still in force.

Signed-off-by: junnncct1106 <jun.514114.ee10@nycu.edu.tw>
One command (scripts/check_es8311_upstream_gate.sh) reports the live state of
the gating PRs (#110205, #107655, #107660) and prints a GATE OPEN / STILL
HOLDING verdict, so the hold (ADR 0004) can be re-checked instantly.

Signed-off-by: junnncct1106 <jun.514114.ee10@nycu.edu.tw>
…comments

Reconcile docs/07_UPSTREAM_PLAN.md item 5 with ADR 0004: replace the stale
"open our own ES8311 PR (ztest 9/9)" framing with the engage-not-compete
decision (ztest 11/11, capture/ADC route as the differentiator vs zephyr#107660,
hold until the gate lifts). Add docs/drafts/es8311-upstream-comments.md holding
the parked, ready-to-post issue-thc1006#7 framing note (Draft A) and the trimmed
upstream engagement comment (Draft B: mute-reg 0x31 confirmation +
hardware-check
offer), plus a submission-time pointer to the readiness checklist.

Re-verified 2026-06-11: checkpatch 0 errors / 0 warnings under Zephyr's
.checkpatch.conf, twister native_sim 11/11; hold conditions still in force
(#107655 and #110205 unmerged, #107660 stalled, no live successor PR).

Signed-off-by: junnncct1106 <jun.514114.ee10@nycu.edu.tw>
The monitor printed GATE OPEN when #110205 OR #107655 merged, but ADR 0004's
gate is #107655 (base board) merged AND the ES8311 work resuming on a live PR.
Our own board PR (#110205) merging alone would falsely open the gate.

- Gate is now AND: base #107655 MERGED *and* a live successor PR exists;
  #110205 dropped from the trigger (kept only as an exclude).
- Live successor = an open es8311 PR that is neither the stalled original
  (#107660) nor one of ours (#110205 / our authors).
- Merge detection now trusts state==MERGED plus an ISO-8601 mergedAt, so a
  closed-unmerged PR cannot count as merged.
- field() returns empty (not "?") on any gh error, so an outage / auth gap /
  rate-limit can no longer be mistaken for a merged PR and false-open the gate.

Verified offline against four scenarios (base+successor -> OPEN; base only,
our-board-only, and gh-down -> all HOLDING) and live against the tracker
(STILL HOLDING; #107655/#107660/#110205 unmerged, no live successor).

Signed-off-by: junnncct1106 <jun.514114.ee10@nycu.edu.tw>
Covers the four cases from the PR thc1006#16 review with a mock gh (no network,
no auth): base #107655 merged + a live successor -> GATE OPEN; base only,
our-board-#110205-only, and gh-down -> all STILL HOLDING. Guards against the
AND-gate and the false-merge ("?") regressions from reverting.

Signed-off-by: junnncct1106 <jun.514114.ee10@nycu.edu.tw>
@thc1006 thc1006 force-pushed the chore/es8311-upstream-readiness branch from c5b24e1 to 7914060 Compare June 11, 2026 09:07
@thc1006

thc1006 commented Jun 11, 2026

Copy link
Copy Markdown
Owner

Heads-up: with your "allow edits from maintainers" on (and your OK), I pushed a fix straight to this branch to clear the red compliance check — it's green now, all 10 checks pass and the PR is mergeable.

Commit messages only — no file content changed (git diff against your old head is empty):

  • Identity (DCO): all 5 commits were authored by you but signed off as Hsiu-Chi Tsai, so Zephyr's Identity check rejected them (author must match a sign-off). Changed each to your own — Signed-off-by: junnncct1106 <jun.514114.ee10@nycu.edu.tw>. That is the correct DCO: you sign your own work.
  • Gitlint T1: the ...(AND-gate, robust merge check) title was 80 chars (>72) -> fix(#7): tighten ES8311 gate to ADR 0004 (AND-gate).
  • Gitlint B1: one 81-char body line in the "align upstream plan" commit -> wrapped.

History was rewritten, so re-sync before you push anything else:

git fetch origin
git reset --hard origin/chore/es8311-upstream-readiness

Non-blocking, left for you (none of these fail CI):

  • The readiness checklist still says "DCO ready — commits already carry Signed-off-by: Hsiu-Chi Tsai" — that is the exact pattern that just failed Identity. For the real upstream ES8311 PR the commits have to be signed off by their actual author, so that line is worth rewording.
  • The gate script's "live successor" matches any open es8311 PR that is not #107660/ours — it will also match a tangential es8311 mention, so eyeball the printed candidate before acting on a GATE OPEN.
  • 5 checkpatch warnings (long lines in the two shell scripts) — cosmetic.
  • The offline gate test is not wired into verify.sh/CI, so it will not catch regressions automatically.

One for next time: verify.sh does not run the compliance gate — running check_compliance.py -m Gitlint -m Identity (or the Docker compliance job) locally would have caught both before the push.

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