New function modbus_reply_callback() #319

frodete · 2016-03-31T14:52:53Z

The modbus_reply_callback() function is modelled closely on modbus_reply(),
but should be used when more complex actions are required for handling
Modbus functions that what is possible with the current mapping scheme.

The modbus_reply_callback() function is modelled iclosely on modbus_reply(), but should be used when more complex actions are required for handling Modbus functions that what is possible with the current mapping scheme.

stephane · 2016-05-18T11:57:31Z

Thank you for your patch and the very good documentation.
The patch duplicates too many lines of code.

I think we can achieve the same behaviour with #323.

It would be nice if you could participate to this new feature (tests, docs, whatever), I'm intend to talk about that on the mailing list... stay tuned.

frodete · 2016-05-18T17:07:02Z

It has been a while since I coded this together for a project and only recently managed to push it. I agree that there are some duplication in the code. However, when I started to make this I did try to look at some way to reduce the duplication. There were a few reasons why I ended up with what I did:

I didn't want to break existing code base. Not a real reason - unless you'r on a deadline. :)
I didn't want to pollute the code with additional branches which could adversely affect the branch predictor (i.e. less code, but worse performance).
My initial attempts produced code which was much worse from a maintenance perspective.
All in all, the total number of identical code lines between the two functions are relatively low - there are probably more duplications internally in each functions, like the error handling.

I am no longer involved in the project which spawned this patch, but I will try to contribute in any way I can.

msinttypes moved to GitHub, so change URL accordingly.

Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>

Signed-off-by: Stéphane Raimbault <stephane.raimbault@gmail.com>

Related to 52ab1bb. The arguments have been changed (see documentation). https://groups.google.com/d/msg/libmodbus/aXO8nBzW4Ew/uVGTDmvvBAAJ

Inspired by etactica@56b2526

Thanks to Karl Palsson.

Thanks to Rüdiger Ranft for the idea.

Thanks to paperwork

stephane#248

Making all in src CC modbus.lo CC modbus-data.lo modbus-data.c:51:5: attention : #warning "Fallback on C functions for bswap_16" [-Wcpp] # warning "Fallback on C functions for bswap_16" ^ modbus-data.c:52:24: erreur: redefinition of ‘bswap_16’ static inline uint16_t bswap_16(uint16_t x)

commit 73a88a7.

code.

frodete · 2016-05-29T13:55:56Z

I tried pushing four changes to this fork to address your concern, but it looks like I ended up with a merge mess. Does the changes make sense or should I start from scratch?

@frodete

Based on @frodete's proposal (stephane#319) a more minimalistic implementation of a callback-based reply-function. The old mapping-implementation is now based on this version. As a nice side-effect some DRY code optimizations could have been done.

@frodete

Based on @frodete's proposal (stephane#319) a more minimalistic implementation of a callback-based reply-function. The old mapping-implementation is now based on this version. As a nice side-effect some DRY code optimizations could have been done.

@frodete

Based on @frodete's proposal (stephane#319) a more minimalistic implementation of a callback-based reply-function. The old mapping-implementation is now based on this version. As a nice side-effect some DRY code optimizations could have been done.

@frodete

Based on @frodete's proposal (stephane#319) a more minimalistic implementation of a callback-based reply-function. The old mapping-implementation is now based on this version. As a nice side-effect some DRY code optimizations could have been done.

@frodete

Based on @frodete's proposal (stephane#319) this implementation adds a a callback-based reply-functionality. The old mapping-implementation is now based on this version. As a nice side-effect some DRY code optimizations could have been done. Also adds a MODBUS_SLAVE_ACCEPT_ALL "address", which can be used with modbus_set_slave(). This makes the modbus_receive()-function accept all requests, independently of the slave-address found in the decoded-request. Useful for multi-slave-implementation using libmodbus.

@frodete

Based on @frodete's proposal (stephane#319) this implementation adds a a callback-based reply-functionality. The old mapping-implementation is now based on this version. As a nice side-effect some DRY code optimizations could have been done. Also adds a MODBUS_SLAVE_ACCEPT_ALL "address", which can be used with modbus_set_slave(). This makes the modbus_receive()-function accept all requests, independently of the slave-address found in the decoded-request. Useful for multi-slave-implementation using libmodbus.

@frodete

Based on @frodete's proposal (stephane#319) this implementation adds a a callback-based reply-functionality. The old mapping-implementation is now based on this version. As a nice side-effect some DRY code optimizations could have been done. Also adds a MODBUS_SLAVE_ACCEPT_ALL "address", which can be used with modbus_set_slave(). This makes the modbus_receive()-function accept all requests, independently of the slave-address found in the decoded-request. Useful for multi-slave-implementation using libmodbus.

@frodete

Based on @frodete's proposal (stephane#319) this implementation adds a a callback-based reply-functionality. The old mapping-implementation is now based on this version. As a nice side-effect some DRY code optimizations could have been done. Also adds a MODBUS_SLAVE_ACCEPT_ALL "address", which can be used with modbus_set_slave(). This makes the modbus_receive()-function accept all requests, independently of the slave-address found in the decoded-request. Useful for multi-slave-implementation using libmodbus.

@frodete

Based on @frodete's proposal (stephane#319) this implementation adds a a callback-based reply-functionality. The old mapping-implementation is now based on this version. As a nice side-effect some DRY code optimizations could have been done. Also adds a MODBUS_SLAVE_ACCEPT_ALL "address", which can be used with modbus_set_slave(). This makes the modbus_receive()-function accept all requests, independently of the slave-address found in the decoded-request. Useful for multi-slave-implementation using libmodbus.

@frodete

Based on @frodete's proposal (stephane#319) this implementation adds a a callback-based reply-functionality. The old mapping-implementation is now based on this version. As a nice side-effect some DRY code optimizations could have been done. Also adds a MODBUS_SLAVE_ACCEPT_ALL "address", which can be used with modbus_set_slave(). This makes the modbus_receive()-function accept all requests, independently of the slave-address found in the decoded-request. Useful for multi-slave-implementation using libmodbus.

@frodete

Based on @frodete's proposal (stephane#319) this implementation adds a a callback-based reply-functionality. The old mapping-implementation is now based on this version. As a nice side-effect some DRY code optimizations could have been done. Also adds a MODBUS_SLAVE_ACCEPT_ALL "address", which can be used with modbus_set_slave(). This makes the modbus_receive()-function accept all requests, independently of the slave-address found in the decoded-request. Useful for multi-slave-implementation using libmodbus.

@frodete

Based on @frodete's proposal (stephane#319) this implementation adds a a callback-based reply-functionality. The old mapping-implementation is now based on this version. As a nice side-effect some DRY code optimizations could have been done. Also adds a MODBUS_SLAVE_ACCEPT_ALL "address", which can be used with modbus_set_slave(). This makes the modbus_receive()-function accept all requests, independently of the slave-address found in the decoded-request. Useful for multi-slave-implementation using libmodbus.

@frodete

Based on @frodete's proposal (stephane#319) this implementation adds a a callback-based reply-functionality. The old mapping-implementation is now based on this version. As a nice side-effect some DRY code optimizations could have been done. Also adds a MODBUS_SLAVE_ACCEPT_ALL "address", which can be used with modbus_set_slave(). This makes the modbus_receive()-function accept all requests, independently of the slave-address found in the decoded-request. Useful for multi-slave-implementation using libmodbus.

@frodete

Based on @frodete's proposal (stephane#319) this implementation adds a a callback-based reply-functionality. The old mapping-implementation is now based on this version. As a nice side-effect some DRY code optimizations could have been done. Also adds a MODBUS_SLAVE_ACCEPT_ALL "address", which can be used with modbus_set_slave(). This makes the modbus_receive()-function accept all requests, independently of the slave-address found in the decoded-request. Useful for multi-slave-implementation using libmodbus.

@frodete

Based on @frodete's proposal (stephane#319) this implementation adds a a callback-based reply-functionality. The old mapping-implementation is now based on this version. As a nice side-effect some DRY code optimizations could have been done. Also adds a MODBUS_SLAVE_ACCEPT_ALL "address", which can be used with modbus_set_slave(). This makes the modbus_receive()-function accept all requests, independently of the slave-address found in the decoded-request. Useful for multi-slave-implementation using libmodbus.

@frodete

Based on @frodete's proposal (stephane#319) this implementation adds a a callback-based reply-functionality. The old mapping-implementation is now based on this version. As a nice side-effect some DRY code optimizations could have been done. Also adds a MODBUS_SLAVE_ACCEPT_ALL "address", which can be used with modbus_set_slave(). This makes the modbus_receive()-function accept all requests, independently of the slave-address found in the decoded-request. Useful for multi-slave-implementation using libmodbus.

New function modbus_reply_callback()

746d3b4

The modbus_reply_callback() function is modelled iclosely on modbus_reply(), but should be used when more complex actions are required for handling Modbus functions that what is possible with the current mapping scheme.

yegorich and others added 27 commits May 29, 2016 14:55

Change msinttypes URL to GitHub repository

817e51a

msinttypes moved to GitHub, so change URL accordingly.

Add Jakob to AUTHORS with Contributor License Agreement

0214a9a

Add an option to disable tests compilation

61854b2

Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>

Fix address range in random-test-client

9478c33

Signed-off-by: Stéphane Raimbault <stephane.raimbault@gmail.com>

Change API modbus_mapping_(offsets_new) to (new_start_address)

7d61d48

Related to 52ab1bb. The arguments have been changed (see documentation). https://groups.google.com/d/msg/libmodbus/aXO8nBzW4Ew/uVGTDmvvBAAJ

Use new modbus_mapping_new_start_address in unit tests

fcdfd63

Add more tests for new modbus_mapping_new_start_address

b5f77b7

Fix "wildcard address" in TCP IPv6

0892ded

Improve documentation of modbus_report_slave_id

de496be

Inspired by etactica@56b2526

Add bswap macro to compile on OSX

31100e3

Minor

471cfff

Update NEWS file

b2d6069

Slight change to modbus_report_slave_id doc.

adb1177

Thanks to Karl Palsson.

Add links to new modbus_*_float_* functions in index

72ce9a6

Move setting of option inside the relevant conditional group

35761b8

Add ./configure.scan to .gitignore

493bce3

Add unit-tests.sh to tarball

991ec1b

Fix small leak (64 bytes in TCP) in unit-test-client

fd35dab

Github's contributing and issue template files

c1cda79

Rewrite and rename README as README.md in tests/

5bef3cf

Fix CID 69140 - Bad bit shift operation (coverity) in tests

7094e39

CID 69142 - Unchecked return value in unit-test-server

b92c709

CID 69145 - Argument cannot be negative in unit-test-server

d0fdd8b

Fix typo in 3053bd0

e5bf0b3

DRY in modbus_reply by improving response_exception()

5b60cdb

Thanks to Rüdiger Ranft for the idea.

Add debug message on unknown function and new unit test

e1c86dd

Fix handling of invalid function code (closes stephane#315)

5b03897

Thanks to paperwork

stephane and others added 11 commits May 29, 2016 14:55

Another round of DRY in modbus_reply()

e769541

Update configure.ac

786644a

stephane#248

Minor coding conventions on defines

bd999ad

Improve ifdef around bswap_16 for __CYGWIN__

48174a8

C_PROG_RANLIB is rendered obsolete by LT_INIT

4d5dfb0

Rewrite new unit test for invalid function code

474217c

Update according to API changes in response_exception() in

2870019

commit 73a88a7.

Add debug message on unknown function code.

94306ef

Add debug messages on illegal callback responses.

ccfbe0c

Refactored code in modbus_reply_callback() to avoid duplication of

493c4ab

code.

pboettch mentioned this pull request Aug 7, 2017

Add modbus_reply_callback() #396

Closed

pboettch mentioned this pull request Oct 31, 2017

Add modbus_reply_callback() and modbus_set_reply_callback() #408

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

New function modbus_reply_callback() #319

New function modbus_reply_callback() #319

Uh oh!

frodete commented Mar 31, 2016

Uh oh!

stephane commented May 18, 2016

Uh oh!

frodete commented May 18, 2016

Uh oh!

frodete commented May 29, 2016 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

New function modbus_reply_callback() #319

Are you sure you want to change the base?

New function modbus_reply_callback() #319

Uh oh!

Conversation

frodete commented Mar 31, 2016

Uh oh!

stephane commented May 18, 2016

Uh oh!

frodete commented May 18, 2016

Uh oh!

frodete commented May 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

frodete commented May 29, 2016 •

edited

Loading