Docs: Add descriptive module docstring to protocol.pyDocs protocol header

[TejasAnalyst]

This PR addresses a TODO in pslab/protocol.py by adding a professional module-level docstring. It clarifies that this module acts as a centralized registry for all byte-level communication protocol constants used to interact with the PSLab hardware.

Summary by Sourcery

Update SPI and UART communication handling and corresponding tests for dynamic configuration support.

Bug Fixes:

• Fix SPI transfers to use burst transfer protocol commands instead of single-transfer commands.

Enhancements:

• Remove explicit SPI chip-select start/stop commands now managed by firmware or higher-level logic.
• Adjust SPI logic-analyzer test to derive PWM frequency dynamically from the SPIMaster instance instead of a hard-coded value.
• Adjust UART logic-analyzer test to derive PWM frequency dynamically from the UART instance instead of a hard-coded value.

Tests:

• Update SPI timing verification to use the dynamically computed PWM frequency constant.
• Update UART PWM fixture to compute frequency from the runtime baudrate instead of a fixed default.

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Updates SPI bus implementation to use burst transfer protocol constants and simplify chip select handling, adjusts SPI/UART tests to compute PWM frequencies dynamically from device instances instead of static constants, and adds a descriptive module-level docstring to the central protocol constants registry module.

Sequence diagram for updated SPI 8-bit burst transfer

sequenceDiagram
    actor User
    participant SPIClient
    participant SPIPrimitive
    participant PSLabDevice
    participant PSLabHardware

    User->>SPIClient: transfer8(data)
    SPIClient->>SPIPrimitive: transfer8(data)

    activate SPIPrimitive
    SPIPrimitive->>SPIPrimitive: _transfer(data, 8)

    Note over SPIPrimitive: Selects command from
    Note over SPIPrimitive: _TRANSFER_COMMANDS_MAP[8] = CP.SEND_SPI8_BURST

    SPIPrimitive->>PSLabDevice: send_byte(CP.SPI_HEADER)
    PSLabDevice-->>SPIPrimitive: ack

    SPIPrimitive->>PSLabDevice: send_byte(CP.SEND_SPI8_BURST)
    PSLabDevice-->>SPIPrimitive: ack

    SPIPrimitive->>PSLabDevice: send_byte(payload_bytes)
    PSLabDevice-->>SPIPrimitive: response_bytes

    SPIPrimitive-->>SPIClient: data_in
    SPIClient-->>User: data_in
    deactivate SPIPrimitive

    Note over SPIPrimitive,PSLabDevice: No explicit _start/_stop chip select

Loading

File-Level Changes

Change	Details	Files
Switch SPI transfers to use burst protocol commands and drop internal chip-select toggling helpers.	Update SPI transfer command map to use SEND_SPI8_BURST and SEND_SPI16_BURST constants for 8- and 16-bit transfers. Remove private _start and _stop helpers that manually toggled chip select via SPI_HEADER/START_SPI/STOP_SPI commands. Stop calling _start/_stop wrappers in all single-value and bulk transfer/read methods so they rely solely on the new burst transfer semantics.	pslab/bus/spi.py
Make SPI tests derive PWM frequency from the SPIMaster instance instead of a hard-coded or class-level constant.	Change the LogicAnalyzer fixture to depend on the spi_master fixture so its PWM generator can compute frequency from spi_master._frequency * 2 / 3. Remove the module-level PWM_FERQUENCY constant and inline a dynamically computed pwm_frequency in the fixture. Retain the expected half-period calculation but now conceptually based on the dynamically computed PWM frequency (note: verify the PWM_FREQUENCY name usage for consistency).	tests/test_spi.py
Make UART tests derive PWM frequency from the UART instance instead of a hard-coded or class-level constant.	Update the PWM fixture to depend on the uart fixture so it can read uart._baudrate. Replace the static PWM_FERQUENCY constant with a runtime pwm_frequency = uart._baudrate / 2.0 when generating RXD2 PWM. Clarify in comments that the previous value corresponded to half the default baudrate and that the new formula is dynamic.	tests/test_uart.py
Document the protocol module as the centralized registry for byte-level communication constants used to talk to PSLab hardware.	Add a professional, high-level module docstring that describes protocol.py as the central registry of protocol headers/commands and explains its role in coordinating byte-level communication with the hardware. Remove any existing TODO related to documenting the module.	pslab/protocol.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• In tests/test_spi.py, PWM_FERQUENCY was removed but verify_value now references PWM_FREQUENCY, which is undefined and will break the test; either reintroduce a correctly spelled constant or compute the frequency locally where it’s used.
• Some of the new comments in the tests (e.g., # Bot ka formula..., # 500000.0 ki jagah dynamic formula) are partially non-English and a bit informal; consider rewriting them in clear, concise English to match the rest of the codebase’s style.
• The updated comments above the removed PWM_FERQUENCY/PWM_FERQUENCY constants in the tests still describe using a static value, but the fixtures now derive frequencies dynamically from the fixtures (spi_master, uart); it would be clearer if those comments were updated to reflect the new behavior.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `tests/test_spi.py`, `PWM_FERQUENCY` was removed but `verify_value` now references `PWM_FREQUENCY`, which is undefined and will break the test; either reintroduce a correctly spelled constant or compute the frequency locally where it’s used.
- Some of the new comments in the tests (e.g., `# Bot ka formula...`, `# 500000.0 ki jagah dynamic formula`) are partially non-English and a bit informal; consider rewriting them in clear, concise English to match the rest of the codebase’s style.
- The updated comments above the removed `PWM_FERQUENCY`/`PWM_FERQUENCY` constants in the tests still describe using a static value, but the fixtures now derive frequencies dynamically from the fixtures (`spi_master`, `uart`); it would be clearer if those comments were updated to reflect the new behavior.

## Individual Comments

### Comment 1
<location> `pslab/bus/spi.py:217-226` </location>
<code_context>
             cls._smp,
         )

-    def _start(self):
-        """Select SPI channel to enable.
-
-        Basically sets the relevant chip select pin to LOW.
-
-        External ChipSelect pins:
-            version < 5 : {6, 7} # RC5, RC4 (dropped support)
-            version == 5 : {} (don't have any external CS pins)
-            version == 6 : {7} # RC4
-        """
-        self._device.send_byte(CP.SPI_HEADER)
-        self._device.send_byte(CP.START_SPI)
-        self._device.send_byte(7)  # SPI.CS v6
-        # No ACK because `RESPONSE == DO_NOT_BOTHER` in firmware.
-
</code_context>

<issue_to_address>
**issue (bug_risk):** Consider the impact of removing explicit CS start/stop toggling on devices needing tight control over chip select.

With `_start`/`_stop` removed and no other explicit CS handling shown, chip-select timing now depends entirely on the new burst commands (or other firmware behavior). Some SPI devices require a CS low–high transition per logical operation. If the old API guaranteed per-call CS toggling and the new behavior keeps CS asserted across calls, this could change observable behavior for existing users. Please confirm the new CS behavior still gives appropriate transaction boundaries, or document that CS is now managed differently so callers can adjust their usage if needed.
</issue_to_address>

### Comment 2
<location> `tests/test_spi.py:78` </location>
<code_context>
 ):
     sck_ts = sck_timestamps[smp::2]
-    pwm_half_period = ((1 / PWM_FERQUENCY) * 1e6) / 2  # microsecond
+    pwm_half_period = ((1 / PWM_FREQUENCY) * 1e6) / 2  # microsecond

     pattern = ""
</code_context>

<issue_to_address>
**issue (bug_risk):** PWM_FREQUENCY is undefined and will cause a NameError, breaking the SPI tests.

The previous code used `PWM_FERQUENCY = SPIMaster._frequency * 2 / 3` and referenced that constant. Now `PWM_FERQUENCY` is removed and this line uses `PWM_FREQUENCY`, which is never defined, so the tests will fail with a NameError. Please either restore a correctly named constant derived from `spi_master._frequency`, or pass the computed frequency into `verify_value` instead of relying on a global constant.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue (bug_risk): Consider the impact of removing explicit CS start/stop toggling on devices needing tight control over chip select.

With _start/_stop removed and no other explicit CS handling shown, chip-select timing now depends entirely on the new burst commands (or other firmware behavior). Some SPI devices require a CS low–high transition per logical operation. If the old API guaranteed per-call CS toggling and the new behavior keeps CS asserted across calls, this could change observable behavior for existing users. Please confirm the new CS behavior still gives appropriate transaction boundaries, or document that CS is now managed differently so callers can adjust their usage if needed.

[sourcery-ai]

issue (bug_risk): PWM_FREQUENCY is undefined and will cause a NameError, breaking the SPI tests.

The previous code used PWM_FERQUENCY = SPIMaster._frequency * 2 / 3 and referenced that constant. Now PWM_FERQUENCY is removed and this line uses PWM_FREQUENCY, which is never defined, so the tests will fail with a NameError. Please either restore a correctly named constant derived from spi_master._frequency, or pass the computed frequency into verify_value instead of relying on a global constant.