Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop OUTPUT_PYTHON from 0:UART #427

Merged
merged 1 commit into from
Sep 5, 2022

Conversation

fornellas
Copy link
Contributor

@fornellas fornellas commented Sep 1, 2021

I'm not sure the rationale, but we have 2 UART decoders:

  • 0:UART says it outputs uart, documents it at OUTPUT_PYTHON but does NOT output anything.
    • This means that stacked decoders receive nothing.
  • 1:UART says it outputs uart, documents it at OUTPUT_PYTHON and DOES output it.

This PR removes the wrong declaration from 0:UART so that people cant't attempt to stack decodes for the case where it does not work.

PS: I've cut a PR to add upstream uart decoder which does not have the issue above and supports setting RX & TX #429. Perhaps the fix here is simply removing both problematic 0:UART and 1:UART.

@fornellas
Copy link
Contributor Author

@dreamsource-tai why was this PR closed? I believe this problem still exists on master.

@fornellas
Copy link
Contributor Author

@dreamsource-tai any reason why this PR can't be merged? It's been open for over a year at this point...

@fornellas
Copy link
Contributor Author

@dreamsource-tai any reason why this PR can't be merged? It's been open for over a year at this point. It should be either merged or rejected with a reason why.

PS: perhaps the solution could be simply to remove both broken UART decoders, see discussion here.

@DreamSourceLab
Copy link
Owner

@fornellas
In the software manual, we have pointed out the limitations of decoders starting with 0:
0uart
Your PR is right, we should remove the ourput declaration from 0:UART.

@DreamSourceLab DreamSourceLab merged commit 9676d29 into DreamSourceLab:master Sep 5, 2022
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.

3 participants