Lambda in Protocol/Transaction class

[TheFern2]

Is there any particular reason why one class out of the whole project is using lambda? Project might reach more people that are still running in java 1.7

The only sole reason why the project doesn't compile in java 1.7 is because the lambda function which then gets converted to regular anonymous function, problem is LongBinaryOperation class doesn't exist in 1.7

Modified code, ran Transaction unittest and passed:

static long nextTransaction()
    {

        if(transaction.get() > 0xffffffffL){
            transaction.set(1);
        } else{
            transaction.incrementAndGet();
        }

        return transaction.get();

Original code below:

        return transaction.incrementAndGet(1, new LongBinaryOperator() {
            @Override
            public long applyAsLong(long value, long plus) {
                long result = value + plus;
                if (result > 0xffffffffL)
                    return 1;
                return result;
            }
        });
    }

I will test in the plc lab, anyways my usage doesn't require tons of transactions I highly doubt I will ever get to the max,I just need to read some tags for a backup utility I am building, and all our servers run on 1.7 and I don't foresee IT's updating anytime soon.

[kasemir]

You're mixing lambda and atomic operation.

No, it doesn't have to use a lambda. I'm OK with Java 1.7. But it has to be atomic.

AtomicLong.accumulateAndGet(..) is, well, atomic, as the name applies. if (AtomicLong.get() .. ) ...AtomicLong.incrementAndGet() is not atomic. Another thread could jump in between the get and the increment call. You might not see that for a long time in running unit tests, but it's not atomic.

Java 7 has https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/atomic/AtomicLong.html, so make a pull request that uses that. If not possible, make a suggestion that adds synchronized.

[TheFern2]

private static final AtomicLong transaction = new AtomicLong(0);

Sorry for the mixup, yeah transaction still atomic. Haven't used much lambda to be honest, but in short what I was attempting to say was that when I made the lambda to anonymous function, LongBinaryOperator doesn't exist in 1.7

Will make PR then. By the way just tested with both L7 V20, and L8 V30 controllers and I was able to connect and read tags. I wasn't sure if I would be able to read Program tags since there is a power point of the etherip on the c repo, and specifically says no program tags. I was able to read both controller, and program tags with no issues.

[kasemir]

In Java 7, AtomicLong lacks the accumulateAndGet method. There's incrementAndGet, but I don't see any way with the Java 7 AtomicLong to atomically increment, check if it wraps around at 0xffffffffL, and then start over at 1.

Would have to synchronize, for example like this:

// SYNC on access
private long transaction;

static long nextTransaction()
{
    synchronized(Transaction.class)
    {
        ++transaction;
        if (transaction > 0xffffffffL)
            transaction = 1;
        return transaction;
}

If I do that, the next person will ask: Why are you stuck with Java 7 now that J10 is out? Why are you blocking instead of using non-blocking constructs? ;-)

[kasemir]

As for program tags, in all the setups I've tried we could only access controller tags. In a way that good, you have control over what's accessible. Program tags remain hidden. Beware, though, that those automatically created controller tags for all the I/O modules are, well, controller tags, so you can read/write those. No way to hide them from network access.

[TheFern2]

I'll revise to non-blocking, yeah it is going to 1 after reaching 0xFFFFFFFF. So I just need to work on the non-blocking portion.

Started at:

Reached:

And back to 1:

[TheFern2]

This is what I came up with finally:

private static final AtomicLong transaction = new AtomicLong(0);

    static long nextTransaction()
    {
        synchronized (Transaction.class){
            if(transaction.get() > 0xffffffffL){
                transaction.set(1);
            } else{
                transaction.incrementAndGet();
            }

            return transaction.get();
        }
    }

Will need to research how to properly unittest nonblocking, because I've never done it before, but for my purposes everything works good on my lab. On my utility I only read less than 20 tags, so I don't foresee any issues. Either way I'll make PR, or you can just point to my repo on your readme if someone is interested in downgraded 1.7 and that way there are two options.

A better choice is probably to compile the jar for this repo and name it 1.8, and then compile the 1.7 and place both jar in a folder.

[kasemir]

When you're using synchronized, you can remove the AtomicLong and instead use a plain long because you are blocking in synchronized. Any potential benefit from AtomicLong is lost. So this would do it:

// SYNC on access
private long transaction;
static long nextTransaction()
{
    synchronized(Transaction.class)
    {
        ++transaction;
        if (transaction > 0xffffffffL)
            transaction = 1;
        return transaction;
    }
}