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

Updated minimalmodbus.read_string arg #9

Merged
merged 1 commit into from
Feb 26, 2021

Conversation

limweichiang
Copy link
Contributor

For minimalmodbus.read_string(), the numberOfDecimals was changed to number_of_decimals.

This change took effect as of MinimalModbus 1.0, refer to https://minimalmodbus.readthedocs.io/en/stable/apiminimalmodbus.html#minimalmodbus.Instrument.read_register

Before fixing, the following error was encountered:

$ python3 renogy_rover.py 
Traceback (most recent call last):
  File "renogy_rover.py", line 185, in <module>
    print('Model: ', rover.model())
  File "renogy_rover.py", line 41, in model
    return self.read_string(12, numberOfRegisters=8)
TypeError: read_string() got an unexpected keyword argument 'numberOfRegisters'

"numberOfDecimals argument was changed to number_of_decimals as of MinimalModbus 1.0, refer to https://minimalmodbus.readthedocs.io/en/stable/apiminimalmodbus.html#minimalmodbus.Instrument.read_register
@corbinbs
Copy link
Owner

Hi @limweichiang !

I'm behind on PR reviews! 😅

I hope to catch up on these this weekend.

Thanks for using the library and for sending PRs back!!!!

@limweichiang
Copy link
Contributor Author

No worries, I love that you built this and I didn't need to spend time hacking something else together. Thanks so much for sharing! Lemme know if you need any help with testing etc, I've got a Renogy Adventurer Li 30A in my setup.

@corbinbs
Copy link
Owner

@limweichiang thanks again for the PR!

I've merged #13 which will flip the project over to use poetry as well as pick up all of the latest versions of the project's dependencies (like minimalmodbus)

I'm going to merge this PR and try to get some runtime on these changes against my own 40A Renogy Rover before releasing a new library version.

Thanks!

@corbinbs corbinbs merged commit 5f7fc61 into corbinbs:master Feb 26, 2021
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.

None yet

2 participants