search

steveohara / j2mod

Enhanced Modbus library implemented in the Java programming language
Apache License 2.0
265 stars 111 forks source link

Change hard coded 'value' in ObservableRegister #68

Closed orensharon closed 6 years ago

orensharon commented 6 years ago

I'm using ObservableRegister to determine updates on my slave devices. It calls notifyObservers with hardcoded argument of 'value', I can't inherit from ObservableRegister and overridden setValue methods because they are final. I had to copy whole ObservableRegister class and change the argument to given one - I think it's a better default behavior.

steveohara commented 6 years ago

Not sure what you are asking to change - do you want the methods not to be final or something else? I'm very happy to take pull requests if you have the time to make some changes.

steveohara commented 6 years ago

To be honest, I have never used the Observable stuff and there aren't any unit tests for it (please submit some if you have some time). Whenever we have created a Slave, we have implemented an instance of ProcessImage and overridden the required register interactions that we are supporting. We then add that to the Slave for a specific Unit ID. It's perhaps a little more code, but gives you a lot more flexibility when handling comms with a master.

orensharon commented 6 years ago

Thanks for your quick answer. I can take pull request. I hope to get time for the unit testing soon.

orensharon commented 6 years ago

@steveohara How do I get permission to push into 'dev' branch?

steveohara commented 6 years ago

You need to fork the project, clone your fork to your local machine, make your changes (adding in unit tests :)), test them, push them to you remote fork and then create a Pull Request. This is explained neatly here; https://www.thinkful.com/learn/github-pull-request-tutorial

steveohara commented 6 years ago

Looking at the code, I can see no reason why any public method or class is set to final so I've removed it from them all.

I can only imagine it was applied by the original author in the belief that it would improve performance because I can't see any mutability issues. Even if it did marginally improve performance, the loss of flexibility wouldn't be worth it.