Fix UART passthrough on v6

[bessman]

Summary by Sourcery

Fix UART passthrough on v6 by removing the persist parameter and implementing separate methods for legacy and current firmware versions. Refactor baud rate constants to improve code clarity and compatibility.

Bug Fixes:

• Fix UART passthrough functionality by removing the persist parameter and adjusting the implementation to handle different firmware versions.

Enhancements:

• Introduce separate methods for UART passthrough to support legacy and current firmware versions, improving compatibility.
• Refactor baud rate constants to distinguish between legacy and current implementations, enhancing code clarity.

[sourcery-ai]

Reviewer's Guide by Sourcery

This pull request fixes UART passthrough functionality for version 6 devices and simplifies the interface. The main changes include implementing version-specific methods for UART passthrough, removing the persist parameter, and updating related constants and method signatures. The changes primarily affect the ScienceLab class, protocol constants, and UART bus handling.

File-Level Changes

Files	Changes
pslab/sciencelab.py	Implement version-specific UART passthrough methods
pslab/sciencelab.py	Remove persist parameter from enable_uart_passthrough method
pslab/sciencelab.py	Add _get_brgval static method for calculating baud rate generator value
pslab/protocol.py	Update UART-related constants and rename legacy constants
pslab/bus/uart.py	Modify UART write behavior based on firmware version

---

Tips

• Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
• Continue your discussion with Sourcery by replying directly to review comments.
• You can change your review settings at any time by accessing your dashboard:
  • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
  • Change the review language;
• You can always contact us if you have any questions or feedback.

[sourcery-ai]

Hey @bessman - I've reviewed your changes - here's some feedback:

Overall Comments:

• Could you explain the removal of the 'persist' parameter and the associated timeout feature? This change might affect existing functionality.
• What's the reason for changing the 'passthrough' value from 15 to 12 in the v6 implementation? A brief comment explaining this would be helpful.
• Consider adding a comment to explain the baudrate calculation (((64e6 / baudrate) / 4) - 1). This would improve code readability.

Here's what I looked at during the review

• 🟡 General issues: 1 issue found
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟢 Complexity: all looks good
• 🟢 Documentation: all looks good

---

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 to tell me if it was helpful.}

[bessman]

@sourcery-ai review

[sourcery-ai]

Hey @bessman - I've reviewed your changes - here's some feedback:

Overall Comments:

• Could you provide more context on the removal of the 'persist' parameter in the enable_uart_passthrough method? Are there any use cases that might be affected by this change?
• In the _write_byte method, the ACK check has been removed for newer firmware versions. Can you explain the reasoning behind this change and confirm that it doesn't introduce any potential race conditions or error scenarios?

Here's what I looked at during the review

• 🟢 General issues: all looks good
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟡 Complexity: 1 issue found
• 🟢 Documentation: all looks good

---

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 to tell me if it was helpful.}

[bessman]

could you provide more context on the removal of the 'persist' parameter in the enable_uart_passthrough method? Are there any use cases that might be affected by this change?

The legacy firmware has an option to exit passthrough mode and return to application mode if no traffic is passed through in a certain time. The timeout is not configurable, and is based on letting the firmware watchdog hard-reset the device. This is not a good approach because it may leave the device in an unknown state.

The new firmware does not have this option. The persist argument is therefore removed in order to keep the interface simple and to prevent users of the legacy firmware from using a reset method which is not recommended.

In the _write_byte method, the ACK check has been removed for newer firmware versions. Can you explain the reasoning behind this change and confirm that it doesn't introduce any potential race conditions or error scenarios?

Keeping the ACK byte to avoid race conditions where the host writes too quickly is a good point. But that would require a change to the firmware, which will have to be handled in separate pull requests to both pslab-firmware and pslab-python. The scope of the current PR is to support the current firmware behavior.