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

Can't read multi-word values #6

Closed
tjhowse opened this issue Jul 20, 2020 · 10 comments
Closed

Can't read multi-word values #6

tjhowse opened this issue Jul 20, 2020 · 10 comments
Labels
enhancement New feature or request

Comments

@tjhowse
Copy link
Owner

tjhowse commented Jul 20, 2020

It would be nice to support 32/64 bit numbers and fixed-length strings.

@tjhowse tjhowse added the enhancement New feature or request label Jul 20, 2020
@tjhowse
Copy link
Owner Author

tjhowse commented Jul 20, 2020

Also types: int16, uint16, int32, uint32, int64, uint64, String(16), String(32), etc.

@tjhowse
Copy link
Owner Author

tjhowse commented Jan 12, 2021

For starters I think the best way to handle this would be to add int32, uint32, int64 and uint64 to the allowed types list, and have the mqtt_interface.__init__ method derive the length and add it as a key to the registers map. Then we'd need to call self._mb.add_monitor_register for the required additional addresses past the starting address in the YAML entry. Then mqtt_interface.poll can use the length field to stitch together a few register values into the multi-word value before doing the type conversion/scaling/etc. Then we'd need a few simple unit tests for reading/writing each new type.

@icypete
Copy link
Contributor

icypete commented Jan 14, 2021

I have written some code that adds int32, uint32, int64 and uint64 types. Tested with SG8K-D and looks to be working well. Didn't break anything and signed 32bit values like Energy Meter Power work now;

image

Code is minimal. I haven't used github much but when I work out making a branch I'll add the code for review...

@icypete
Copy link
Contributor

icypete commented Jan 14, 2021

here are the code changes I made to add multi byte support.

i had to comment out one line for the masking, but it should be easliy fixed

multi byte support.zip

@icypete
Copy link
Contributor

icypete commented Jan 14, 2021

ok, i have fixed up the masking I commented out in the code i provided above. this zip file read's multi-byte addresses and lets masks work still.

multi byte support - with masks.zip

@tjhowse
Copy link
Owner Author

tjhowse commented Jan 14, 2021

Thanks very much! This is awesome! I'll set aside some time this weekend to get this merged in.

I'm happy to do it on my end, but would you like a hand walking through the github pull request process? This way your profile will show you as a contributor to this repo, and it'll be your commit to the main branch containing the changes rather than mine. Up to you!

@icypete
Copy link
Contributor

icypete commented Jan 15, 2021

cool, no problem if you want to merge any of that code yourself. this is a cool little project though and i'd love to be able to commit code. i'll read up on how to do that (if you have a github-101 cheat sheet let me know)

@tjhowse
Copy link
Owner Author

tjhowse commented Jan 15, 2021

Here's a decent guide to the overall idea behind the pull-request-based workflow: https://www.atlassian.com/git/tutorials/making-a-pull-request

The short version: You "fork" a repository to create a "downstream" copy of it under your own github account. You clone that fork to your local machine, make your changes and then push them back up to your github fork. You can then raise a request to pull the changes from your fork back into the "upstream" fork. The maintainer (me, in this case) and the contributor (you) have a discussion about the change in the pull request web interface until both parties are happy with the changes. At this point the maintainer merges the pull request into the upstream fork.

@icypete
Copy link
Contributor

icypete commented Jan 15, 2021

nice, seems straight forward. I have cloned and forked the repository. i'll have a go at submitting a pull request today.

@tjhowse
Copy link
Owner Author

tjhowse commented Aug 24, 2022

Closed by #29

@tjhowse tjhowse closed this as completed Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants