Make CRSF throttle_channel a const, add channel extraction test by kaidokert · Pull Request #37 · kaidokert/rm32

kaidokert · 2026-05-01T19:18:54Z

Summary

throttle_channel field → pub const THROTTLE_CHANNEL: usize = 2
Document C/Rust convention difference for AETR channel mapping
New test: builds CRSF frame with channel 2 = 1000, verifies extraction + throttle mapping = 1034

Test plan

227 unit tests pass (1 new)
46 blackbox tests pass, 1 xfail
All 4 MCU cross-builds pass
Clippy clean

Summary by CodeRabbit

Tests
- Added validation tests ensuring CRSF frame decoding and correct throttle mapping from received channels.
Refactor
- Simplified throttle selection to a fixed channel constant instead of runtime configuration, reducing setup complexity and potential misconfiguration.

throttle_channel was a pub struct field set to 2 and never modified. Replace with pub const THROTTLE_CHANNEL: usize = 2. Document the C/Rust convention difference (C uses index 1, Rust uses index 2 — both refer to AETR channel 3 / Throttle). Add test that builds a CRSF frame with a known value in channel 2, parses it, and verifies the throttle mapping produces the correct ESC throttle value (1034 for CRSF value 1000).

sourcery-ai

Sorry @kaidokert, you have reached your weekly rate limit of 1500000 diff characters.

Please try again later or upgrade to continue using Sourcery

coderabbitai · 2026-05-01T19:19:05Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bcd01da4-4ab1-4bd4-a707-bb86a2b90318

📥 Commits

Reviewing files that changed from the base of the PR and between 4ffbef1 and 785c1d6.

📒 Files selected for processing (1)

rm32/src/crsf.rs

🚧 Files skipped from review as they are similar to previous changes (1)

rm32/src/crsf.rs

📝 Walkthrough

Walkthrough

Throttle channel selection moved from a runtime-configurable CrsfParser field to a compile-time constant THROTTLE_CHANNEL = 2; ISR code now references this constant. A unit test verifies decoding and mapping for channel 2.

Changes

Cohort / File(s)	Summary
CRSF Parser `rm32/src/crsf.rs`	Removed `pub throttle_channel: u8` from `CrsfParser`; added `pub const THROTTLE_CHANNEL: usize = 2`; added unit test that builds a frame with channel 2 = 1000 and asserts unpack/mapping results.
ISR Throttle Handling `rm32_stm32/src/isr_handlers.rs`	Replaced runtime `state.crsf.throttle_channel` lookup with `rm32::crsf::THROTTLE_CHANNEL` when extracting and converting throttle from `channels`.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I hopped through frames and found,
A number fixed, no settings bound.
Channel two now leads the dance,
Packets parse with clearer stance.
Cheers from my whiskers, small and proud!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the two main changes: converting the throttle_channel to a const and adding a channel extraction test.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch pr31_1

_{Review rate limit: 3/5 reviews remaining, refill in 15 minutes and 32 seconds.}

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

codecov · 2026-05-01T19:19:33Z

❌ 2 Tests Failed:

Tests completed	Failed	Passed	Skipped
273	2	271	1

View the full list of 2 ❄️ flaky test(s)

tests.blackbox.test_vectors::test_vector[signal_timeout]

Flake rate in main: 100.00% (Passed 0 times, Failed 52 times)

Stack Traces | 0.003s run time

harness = <harness.AM32Harness object at 0x7f55d2750fe0>
vector_file = PosixPath('.../blackbox/vectors/signal_timeout.txt')

    @pytest.mark.parametrize(
        "vector_file",
        get_vector_files(),
        ids=lambda p: p.stem,
    )
    def test_vector(harness, vector_file):
        """Run a single test vector file."""
        if vector_file.stem in XFAIL_VECTORS:
            pytest.xfail(f"Known Rust vs C difference: {vector_file.stem}")
>       run_test_vectors(harness, vector_file)

tests/blackbox/test_vectors.py:54: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

harness = <harness.AM32Harness object at 0x7f55d2750fe0>
vectors_file = PosixPath('.../blackbox/vectors/signal_timeout.txt')

    def run_test_vectors(harness, vectors_file):
        """
        Run test vectors from a file.
    
        Format:
            # Comments start with #
            # Blank lines are ignored
    
            @config
            key=value
            key=value
    
            @sequence
            # tick_spec | inputs | assertions
            tick 1      | throttle=0          |
            ticks 20000 | throttle=0          | armed=1
            tick 1      | throttle=500        | running=1 duty_cycle>0
        """
        import re
    
        section = None
        config_lines = []
        sequence_lines = []
    
        with open(vectors_file, encoding="utf-8") as f:
            for line in f:
                line = line.strip()
                if not line or line.startswith("#"):
                    continue
                if line == "@config":
                    section = "config"
                    continue
                if line == "@sequence":
                    section = "sequence"
                    continue
                if section == "config":
                    config_lines.append(line)
                elif section == "sequence":
                    sequence_lines.append(line)
    
        # Apply config
        harness.reset()
        for cl in config_lines:
            if "=" not in cl:
                continue
            k, v = cl.split("=", 1)
            harness.config(**{k.strip(): v.strip()})
    
        # Operator dispatch table
        ops = {
            "=": lambda a, b: a == b,
            ">": lambda a, b: a > b,
            "<": lambda a, b: a < b,
            ">=": lambda a, b: a >= b,
            "<=": lambda a, b: a <= b,
        }
    
        # Run sequence
        results = []
        for seq_line in sequence_lines:
            # Inline config commands
            if seq_line.startswith("config "):
                kvs = seq_line[7:]
                for token in kvs.split():
                    if "=" in token:
                        k, v = token.split("=", 1)
                        harness.config(**{k: v})
                continue
    
            # Inline commands
            if seq_line == "load_eeprom":
                harness.load_eeprom()
                continue
    
            parts = [p.strip() for p in seq_line.split("|")]
            cmd_part = parts[0]
            input_part = parts[1] if len(parts) > 1 else ""
            assert_part = parts[2] if len(parts) > 2 else ""
    
            # Parse inputs
            inputs = {}
            for token in input_part.split():
                if "=" in token:
                    k, v = token.split("=", 1)
                    inputs[k] = int(v)
    
            # Execute
            tokens = cmd_part.split()
            cmd = tokens[0]
            if cmd == "tick":
                state = harness.tick(**inputs)
            elif cmd == "ticks":
                n = int(tokens[1])
                state = harness.ticks(n, **inputs)
            elif cmd == "gcr_encode":
                com_time = int(tokens[1])
                state = harness.gcr_encode(com_time, **inputs)
            else:
                raise ValueError(f"Unknown command: {cmd}")
    
            # Check assertions
            if assert_part:
                for assertion in assert_part.split():
                    # Support: key=value, key>value, key<value, key>=value, key<=value
                    # Value can be numeric or string (for gcr= comma-separated buffers)
                    m = re.match(r"(\w+)(>=|<=|>|<|=)([\w,.-]+)", assertion)
                    if not m:
                        raise ValueError(f"Bad assertion: {assertion}")
                    key, op, expected_str = m.group(1), m.group(2), m.group(3)
                    actual = state.get(key)
                    if actual is None:
>                       raise KeyError(f"Key '{key}' not in state")
E                       KeyError: "Key 'armed' not in state"

tests/blackbox/harness.py:255: KeyError

tests.blackbox.test_vectors::test_vector[telemetry]

Flake rate in main: 100.00% (Passed 0 times, Failed 52 times)

Stack Traces | 0.002s run time

harness = <harness.AM32Harness object at 0x7f55d2753170>
vector_file = PosixPath('.../blackbox/vectors/telemetry.txt')

    @pytest.mark.parametrize(
        "vector_file",
        get_vector_files(),
        ids=lambda p: p.stem,
    )
    def test_vector(harness, vector_file):
        """Run a single test vector file."""
        if vector_file.stem in XFAIL_VECTORS:
            pytest.xfail(f"Known Rust vs C difference: {vector_file.stem}")
>       run_test_vectors(harness, vector_file)

tests/blackbox/test_vectors.py:54: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

harness = <harness.AM32Harness object at 0x7f55d2753170>
vectors_file = PosixPath('.../blackbox/vectors/telemetry.txt')

    def run_test_vectors(harness, vectors_file):
        """
        Run test vectors from a file.
    
        Format:
            # Comments start with #
            # Blank lines are ignored
    
            @config
            key=value
            key=value
    
            @sequence
            # tick_spec | inputs | assertions
            tick 1      | throttle=0          |
            ticks 20000 | throttle=0          | armed=1
            tick 1      | throttle=500        | running=1 duty_cycle>0
        """
        import re
    
        section = None
        config_lines = []
        sequence_lines = []
    
        with open(vectors_file, encoding="utf-8") as f:
            for line in f:
                line = line.strip()
                if not line or line.startswith("#"):
                    continue
                if line == "@config":
                    section = "config"
                    continue
                if line == "@sequence":
                    section = "sequence"
                    continue
                if section == "config":
                    config_lines.append(line)
                elif section == "sequence":
                    sequence_lines.append(line)
    
        # Apply config
        harness.reset()
        for cl in config_lines:
            if "=" not in cl:
                continue
            k, v = cl.split("=", 1)
            harness.config(**{k.strip(): v.strip()})
    
        # Operator dispatch table
        ops = {
            "=": lambda a, b: a == b,
            ">": lambda a, b: a > b,
            "<": lambda a, b: a < b,
            ">=": lambda a, b: a >= b,
            "<=": lambda a, b: a <= b,
        }
    
        # Run sequence
        results = []
        for seq_line in sequence_lines:
            # Inline config commands
            if seq_line.startswith("config "):
                kvs = seq_line[7:]
                for token in kvs.split():
                    if "=" in token:
                        k, v = token.split("=", 1)
                        harness.config(**{k: v})
                continue
    
            # Inline commands
            if seq_line == "load_eeprom":
                harness.load_eeprom()
                continue
    
            parts = [p.strip() for p in seq_line.split("|")]
            cmd_part = parts[0]
            input_part = parts[1] if len(parts) > 1 else ""
            assert_part = parts[2] if len(parts) > 2 else ""
    
            # Parse inputs
            inputs = {}
            for token in input_part.split():
                if "=" in token:
                    k, v = token.split("=", 1)
                    inputs[k] = int(v)
    
            # Execute
            tokens = cmd_part.split()
            cmd = tokens[0]
            if cmd == "tick":
                state = harness.tick(**inputs)
            elif cmd == "ticks":
                n = int(tokens[1])
                state = harness.ticks(n, **inputs)
            elif cmd == "gcr_encode":
                com_time = int(tokens[1])
                state = harness.gcr_encode(com_time, **inputs)
            else:
                raise ValueError(f"Unknown command: {cmd}")
    
            # Check assertions
            if assert_part:
                for assertion in assert_part.split():
                    # Support: key=value, key>value, key<value, key>=value, key<=value
                    # Value can be numeric or string (for gcr= comma-separated buffers)
                    m = re.match(r"(\w+)(>=|<=|>|<|=)([\w,.-]+)", assertion)
                    if not m:
                        raise ValueError(f"Bad assertion: {assertion}")
                    key, op, expected_str = m.group(1), m.group(2), m.group(3)
                    actual = state.get(key)
                    if actual is None:
>                       raise KeyError(f"Key '{key}' not in state")
E                       KeyError: "Key 'send_telemetry' not in state"

tests/blackbox/harness.py:255: KeyError

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

gemini-code-assist

Code Review

This pull request refactors the CRSF parser by replacing the configurable throttle_channel field with a global constant THROTTLE_CHANNEL and adds a unit test for channel extraction. Feedback includes a request to clarify documentation regarding channel indexing conventions to ensure parity with the reference C implementation and a suggestion to remove redundant code in the newly added test.

The comment incorrectly claimed C index 1 mapped to the same channel as Rust index 2. In fact, the C variable is declared but never used (CRSF is behind an uncompiled flag). Updated comment to state this. Remove dead payload construction code left over from test iteration.

sourcery-ai Bot reviewed May 1, 2026

View reviewed changes

gemini-code-assist Bot reviewed May 1, 2026

View reviewed changes

Comment thread rm32/src/crsf.rs Outdated

Comment thread rm32/src/crsf.rs Outdated

kaidokert merged commit d88649c into main May 1, 2026
23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make CRSF throttle_channel a const, add channel extraction test#37

Make CRSF throttle_channel a const, add channel extraction test#37
kaidokert merged 2 commits into
mainfrom
pr31_1

kaidokert commented May 1, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

sourcery-ai Bot left a comment

Uh oh!

coderabbitai Bot commented May 1, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented May 1, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

kaidokert commented May 1, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

sourcery-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 2 Tests Failed:

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

kaidokert commented May 1, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 1, 2026 •

edited

Loading

codecov Bot commented May 1, 2026 •

edited

Loading