ornl-epics / etherip

Java lib. for Ethernet/IP (AllenBradley ControlLogix, Compact Logix PLCs)
Eclipse Public License 1.0
112 stars 69 forks source link

Etherip with Netvisor extension #13

Closed laszlopataki closed 7 years ago

laszlopataki commented 7 years ago

The principle of the etherip didn't change, but I extended it with many functionalities. These are tested with an Allen Bradley 1769. If you have any suggest or remark we can discuss it. I am not sure that the formatting rules are the same as original etherip so if you are interested in merging this fork and you are using Eclipse, then please share the exportable formatter XML.

kasemir commented 7 years ago

You've added quite a lot!

You can use the code_format.xml that was just committed.

In addition, in the new files, can you please add a copyright, which may use your organization, and otherwise keeps the EPL?

/*******************************************************************************
 * Copyright (c) 2017 (your site)
 * All rights reserved. This program and the accompanying materials
 * are made available under the terms of the Eclipse Public License v1.0
 * which accompanies this distribution, and is available at
 * http://www.eclipse.org/legal/epl-v10.html
 *******************************************************************************/

In the new files, add yourself as the @author. In the files you changed, you already added some of your @author tags, that's good.

Finally, in the CHANGE_LOG, list what's new:

Thanks!

laszlopataki commented 7 years ago

I committed the changes, please check it.

I had some problem with the format style. I like the gradual code style wich you have used in some methods of Etherip. It represents perfectly the encapsulation process. So I wanted to keep it but I could not get the formatter on it. Finally, I just turned off in that section as well as in the CIPException class. Furthermore, I edited the code format of the enums so now it keeps every value in a new line as it was in the original Etherip project.

I have seen that you do not forget to use the final keyword. Maybe it is a save action in your IDE, but as I know it is not importable so I exported my clean up profile with which this thing could be unified. Please check the updated code_format.xml and clean_up_profile.xml and if you agree then it could be the unified code style of the project.

TcpConnecton and UdpConnecton classes are very similar to your Connection class so I wrote there your name but these are new files so the NETvisor copyright text is on the top.

The CHANGE_LOG was extended and I complemented the @author tags and copyright texts. The copyright can stay EPL.

Let me know if you need anything else. Thanks in advance.

kasemir commented 7 years ago

I like the gradual code style .. represents perfectly the encapsulation process.

I agree that my code style is superior to any automated formatting ;-). I really like balanced quotes, which tend to add one more line to each block of code, but then I make up for that by removing unneeded quotes and line breaks. So this:

void func()
{
   final int i = 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9;
   if (true)
      do_this();
   else
   {
      do_that();
      and_that();
   }
}

instead of that:

void func() {
   final int i =
       1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9;
   if (true) {
      do_this();
   }
   else {
      do_that();
      and_that();
   }
}

Then there are things like the protocol layering which I find more readable when suitably indented, while some automated formatter simply counts characters and adds a line break to keep it under 80 columns.

Haven't found any formatter that does what I need, so I actually don't use them, but since you asked for one, I thought it better to provide something.