Make all ServoState fields private

[kaidokert]

Summary

All 11 ServoState fields made private:

• set_calibration(low, high, neutral, dead_band) replaces 4 field writes
• calibration_required() / high_calibration_set() accessors for transfer.rs
• set_calibration_required() setter for transfer.rs write

Test plan

• 226 unit tests pass
• 46 blackbox tests pass, 1 xfail
• All 4 MCU cross-builds pass
• Clippy clean

Summary by CodeRabbit

• Refactor
  • Improved internal servo calibration state encapsulation through standardized method accessors, enhancing code maintainability without affecting user-facing functionality.

[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]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 975a46c4-24ea-45c9-ab4e-01562e0329df

📥 Commits

Reviewing files that changed from the base of the PR and between a84070f and f391184.

📒 Files selected for processing (3)

• rm32/src/servo.rs
• rm32/src/transfer.rs
• rm32_stm32/src/bin/main.rs

---

📝 Walkthrough

Walkthrough

Calibration state management in ServoState is refactored to encapsulate calibration_required and high_calibration_set fields by making them private. New accessor/mutator methods replace direct field access, and a consolidated set_calibration() method replaces individual threshold assignments at call sites.

Changes

Cohort / File(s)	Summary
Servo state encapsulation rm32/src/servo.rs	Made calibration_required and high_calibration_set fields private; added methods calibration_required(), set_calibration_required(), high_calibration_set(), and set_calibration() for controlled access to calibration thresholds.
Calibration method call updates rm32/src/transfer.rs, rm32_stm32/src/bin/main.rs	Replaced direct field access with method calls (calibration_required(), set_calibration_required()) and consolidated set_calibration() method for setting low/high/neutral/dead-band thresholds instead of individual assignments.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• Make fields private across 7 modules, delete dead TransferActions fields #35: Modifies servo calibration field visibility and related call sites in the same files.
• Make struct fields private, add accessor methods #24: Implements the same encapsulation pattern—making struct fields private and adding accessor/mutator methods.

Poem

🐰 Fields once open, now they hide,
Behind methods, safe inside!
Calibration's guarded well,
Setters, getters weave their spell—
Encapsulation's gentle dance! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Make all ServoState fields private' directly and clearly summarizes the main objective of the pull request—making all 11 ServoState fields private and introducing accessor/setter methods for controlled access.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 pr30_1

---

_{Review rate limit: 4/5 reviews remaining, refill in 12 minutes.}

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

[codecov]

❌ 2 Tests Failed:

Tests completed	Failed	Passed	Skipped
272	2	270	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 50 times)

Stack Traces | 0.002s run time

harness = <harness.AM32Harness object at 0x7fb803109730>
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 0x7fb803109730>
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 50 times)

Stack Traces | 0.002s run time

harness = <harness.AM32Harness object at 0x7fb80310b3e0>
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 0x7fb80310b3e0>
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 encapsulates the ServoState struct by making its fields private and introducing getter and setter methods, updating the logic in transfer.rs and main.rs accordingly. A suggestion was made to reset internal calibration counters and flags within set_calibration_required to ensure a clean state when a new calibration sequence begins.