Added messagetype 0x14 and 0x15 for file-access.

[OliverDDS]

Added 0x14 and 0x15 for File-access. Extended interface with two new functions, to make them useable.

I implemented the message-type 0x14 (Read-general-reference) and 0x15 (Write-general-reference), which offers a file-like access to up to 10 register-areas. To allocate these additional areas, i introduced two new functions, so that the old interface is stable. In modbus_mapping_new_start_address_extend and modbus_mapping_new_extend, an additional parameter uint16_t nb_file_registers[] let you determine the size of the file-register-areas ( 0 for not used files )

Two use the messagetypes, two functions modbus_read_general_reference and modbus_write_general_reference, have been added, which offer the simple and normally used query with one subrequest.

I also extended the unit-tests to verify the implementation.

Take this over to mainline, if you like it.

Best regards, Oliver

[OliverDDS]

Hmm, strange. I checked with clang-3.5 :

> ============================================================================
> Testsuite summary for libmodbus 3.1.4
> ============================================================================
>  TOTAL: 1
>  PASS:  1
>  SKIP:  0
>  XFAIL: 0
>  FAIL:  0
>  XPASS: 0
>  ERROR: 0
> ============================================================================

Where can i get the log from the tests ?

[pboettch]

Thanks for your PR. Adding 0x14 and 0x15 is a good thing.

I have two or three remarks:

1. I think you should consider fixing all your indent and white-space-problems and repush as soon as you can. Ie. no tabs for indentation and alignments. Also there are some incoherent white-space after and before operators.

Do not get desperate, there is a clang-format-file which does quite a good job reproducing the original style. (copy it manually from here if you like: https://github.com/pboettch/libmodbus/blob/clang-format/.clang-format)

2. Add documentation. You need to add man-pages for your new functions inside the doc-directory.

3. When compiling your branch with gcc-7 I'm getting warnings (which is due to the incoherent white-space and indent-stuff):

Making all in src
  CC       modbus.lo
../../src/modbus.c: In function ‘modbus_mapping_new_extend’:
../../src/modbus.c:2189:5: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
     if (nb_file_register[i])
     ^~
../../src/modbus.c:2191:4: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the ‘if’
    memset(mb_mapping->file_registers[i],0,nb_file_register[i] * sizeof(uint16_t));
    ^~~~~~
../../src/modbus.c: In function ‘modbus_mapping_new_start_address_extend’:
../../src/modbus.c:2220:19: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
                   if (nb_file_registers[i])
                   ^~
../../src/modbus.c:2222:25: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the ‘if’
                         memset(mb_mapping->file_registers[i],0,nb_file_registers[i] * sizeof(uint16_t));
                         ^~~~~~

[OliverDDS]

Thanks for the hints, i fixed the indention and also the warning about the if clause.

That's the curse, when working on different PCs, with different OS and different Editors :(

I also added the documentation for the new functions. When writing it, i'm not really happy about the naming "modbus_mapping_new_start_address_extend", but i haven't got a better name for it, and didn't want to change the current function , to stay backward compatible.

[mhei]

I would appreciate if you would restructure this PR: divide it in smaller changesets, each with a meaningful commit message for itself. This makes it easier to review. I would propose e.g. first patch which adds the new core functionality, one which add the documentation and one or two which adjusts the tests (e.g. factor out the argv-related stuff as this could stand independently from your topic changes). Please also squash the whitespace fixes into the "topic" changesets. Please re-use this PR by using git push -f ... in your branch. I also don't like the approach of extending the mapping stuff. Instead we should consider a more general approach by creating something like a Modbus server context, which can hold multiple mappings in parallel and the new file stuff. However, I have for myself not a really good proposal at hand right now...

[OliverDDS]

I squashed everything into one commit now. About splitting it into smaller commits, I not really sure if i got you right, but splitting the documentation and implementation would not help, because that are different files. The main change is in modbus.c and makes only sense in one big blob. Only the argv changes ( around 15 lines in each unit-test source ), could be split, but they are only in, because i have a different setup here and don't like to change the harcoded names everytime :(

[pboettch]

@OliverDDS Your manually made coding-style changes weren't enough, I opened a PR on your repo which includes the changes I got after running clang-format like this, feel free to squash them.

Basically I ran

git diff -U0 HEAD^ | clang-format-diff -p1 -i

with the .clang-format-file in libmodbus/.

Regarding splitting up: I see 4 commits you could create:

1. API changes
2. client-code
3. server-code
4. tests

It is up to the maintainer to squash them after review.

@mhei Regarding a server implementation, I agree, the mapping-stuff is not optimal. Have you seen my updated pull-request (modbus_reply_callback())? It is ready for review. It's big, though.

[OliverDDS]

Another try: -There are several commits now for implementation, documentation, tests and improvement of argv -using clang-format-diff-3.9 with .clang_format to format it correctly.

Hope, this is now, as it should be.

[OliverDDS]

I changed the code according to the review comments

• used c-style comments
• changed naming of the functions and the defines towards the current specification
• updated documentation

[LexusAD]

Will this pull request merged into master or is 0x14 implemented in another branch? Maybe consider solving the merge conflicts

[OliverDDS]

Work was picked up here : https://github.com/stephane/libmodbus/pull/449 and here : https://github.com/stephane/libmodbus/pull/548 So no need to have it three times. If you like this better, feel free to use or solve the merged conflict ( which is trivial) and try to get it merged.