Skip to content

Feature/270 specify transmitter packet size#271

Merged
LorenzzoQM merged 3 commits intodevelopfrom
feature/270-specify-transmitter-packet-size
Jun 5, 2025
Merged

Feature/270 specify transmitter packet size#271
LorenzzoQM merged 3 commits intodevelopfrom
feature/270-specify-transmitter-packet-size

Conversation

@LorenzzoQM
Copy link
Copy Markdown
Contributor

Description

Closes #270

Add the option to specify the transmitterPacketSize. If not specified, it sets it as the instrument baud rate, as previously.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How should this pull request be reviewed?

  • By commit
  • All changes at once

How Has This Been Tested?

By running unit and integration tests.

Checklist

  • I have performed a self-review of my code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation and release notes
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

@LorenzzoQM LorenzzoQM self-assigned this Jun 5, 2025
Copilot AI review requested due to automatic review settings June 5, 2025 18:23
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds a new transmitterPacketSize parameter to the transmitter setup so users can override the default packet size (which remains the instrument baud rate if unspecified).

  • Introduce transmitterPacketSize as an optional argument in setup_transmitter
  • Update logic to use the specified packet size or fall back to the instrument baud rate
  • Update release notes to document the new feature

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/bsk_rl/sim/dyn.py Added optional transmitterPacketSize parameter and conditional logic
docs/source/release_notes.rst Documented transmitterPacketSize option in release notes
Comments suppressed due to low confidence (2)

src/bsk_rl/sim/dyn.py:857

  • The new branch for a user-specified transmitterPacketSize is not covered by existing tests; consider adding a unit test verifying that the override is applied correctly.
self.transmitter.packetSize = transmitterPacketSize  # bits

src/bsk_rl/sim/dyn.py:834

  • Optional is used but not imported; please add from typing import Optional at the top of the file to avoid a NameError.
transmitterPacketSize: Optional[int] = None,

@LorenzzoQM LorenzzoQM marked this pull request as draft June 5, 2025 18:28
@LorenzzoQM LorenzzoQM force-pushed the feature/270-specify-transmitter-packet-size branch from 5c8d0ed to 7dd57c7 Compare June 5, 2025 18:40
@LorenzzoQM LorenzzoQM marked this pull request as ready for review June 5, 2025 18:52
Copy link
Copy Markdown
Contributor

@Mark2000 Mark2000 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Working on fixing the things that broke in BSK to make the tests pass.

@LorenzzoQM LorenzzoQM merged commit d5af266 into develop Jun 5, 2025
9 of 10 checks passed
@LorenzzoQM LorenzzoQM deleted the feature/270-specify-transmitter-packet-size branch June 5, 2025 20:44
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.

Specify transmitter packet size

3 participants