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

fix: check force_speed by absence of property #164

Closed
wants to merge 1 commit into from

Conversation

git-developer
Copy link
Contributor

Fix for #162

@git-developer git-developer marked this pull request as draft March 17, 2024 12:04
@Ayush1325
Copy link
Contributor

Ayush1325 commented Mar 17, 2024

Found the problem. The force_speed property was originally set if -b was specified and was not a cli argument. Since it does not exist as an argument, args.force_speed is always false. It can be fixed as follows:

# Remove default value
 parser.add_argument('-b', '--baud', type=int, help='Baud speed (default = 500000)')

..

force_speed = False
if args.baud:
    force_speed = True
else:
    args.baud = 500000

@git-developer
Copy link
Contributor Author

Thanks for your quick response, @Ayush1325! I'd like to encourage you to open a PR and add the changes there. I will close this one then. The reason is that I cannot judge if the changes make sense or even are syntactically OK (I'm not a python developer).

If that is not acceptable for you, I can add patches to this PR. I'd like to ask you for the exact location and content of the code changes, preferably in patch format.

@git-developer
Copy link
Contributor Author

I noticed one thing about your proposal: the default baud rate of 500000 is already set in open(). I suggest to avoid this duplication.

@git-developer
Copy link
Contributor Author

Closed for #165

@Ayush1325
Copy link
Contributor

I have created a PR: #165

While it is true that it is already set in open(), args.baud is being used at many other places and thus needs to be set. Also since baud is the 2nd argument of the function, the default value cannot be removed there without some refactoring. So I have moved the default baud to a global constant for now.

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