Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MavlinkFieldInfo annotations effected by Comparator definition using Eclipse only IDE #7

Closed
Enlade opened this issue Sep 27, 2018 · 5 comments
Labels
bug Something isn't working verified

Comments

@Enlade
Copy link

Enlade commented Sep 27, 2018

The MavlinkFieldInfo annotations do not seem to be working when using Eclipse only IDE. The problem actually turns out to be an issue with the Comparator in the MavlinkFieldInfo interface. As it is currently defined the Comparator prevents access to the annotations. I do not know why this is the case because the code looks fine to me. However, if I modify the Comparator declaration within the MavlinkFieldInfo interface then everything works fine (as below):

package io.dronefleet.mavlink.annotations;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.util.Comparator;

@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.METHOD})
public @interface MavlinkFieldInfo {	
	
	Comparator<MavlinkFieldInfo> WIRE_COMPARATOR = new Comparator<MavlinkFieldInfo>() {
		@Override
		public int compare(MavlinkFieldInfo a, MavlinkFieldInfo b) {
		    if (a.extension() && !b.extension()) {
		        return 1;
		    }
		    if (!a.extension() && b.extension()) {
		        return -1;
		    }
		
		    if (!a.extension() && a.unitSize() != b.unitSize()) {
		        return b.unitSize() - a.unitSize();
		    }
		    
		    return a.position() - b.position();
		};
	};
	
    int position();
    int unitSize();
    boolean signed() default false;
    int arraySize() default 0;    
    Class<?> enumType() default void.class;    
    boolean extension() default false;    
    String description() default "No description provided";
}

If this above code works in the Maven and Gradle environments then maybe make that change so that it also works without much trouble in the Eclipse only IDE environment. Obviously it is better to distribute the code by Maven, but in some offline development environments Maven or Gradle are difficult to manage, so it would be good for the code to also work within the Eclipse only IDE environment (particularly if it is easy enough to make it so).

@joelkoz
Copy link

joelkoz commented Oct 12, 2018

There is definitely something going on here, and I don't think it is necessarily Eclipse specific. I just opened a separate issue #13 re: the serialization/deserialization not working for me, and while trying to track down the problem, I found myself in the debugger looking at this exact same class. Remembering seeing this issue, I copied @Enlade 's code in place, and suddenly things started working. Also, my "gradle test" now passes. @benbarkay are you sure that the unit tests pass with the other MavlinkFieldInfo code in place? I haven't studied this code closely to see what the difference is, but I can tell you that this version makes all my other problems go away.

@joelkoz
Copy link

joelkoz commented Oct 13, 2018

If this above code works in the Maven and Gradle environments then maybe make that change so that it also works without much trouble in the Eclipse only IDE environment. Obviously it is better to distribute the code by Maven, but in some offline development environments Maven or Gradle are difficult to manage, so it would be good for the code to also work within the Eclipse only IDE environment (particularly if it is easy enough to make it so).

I might add that I downloaded the 1.0.9 and 1.0.10 .jar files from Maven Central, and those FAILED also in my environment. I have no idea if there was some bug in the lambda version that has been fixed in a subsequent version of the JRE, or if there is a bug in the OS X that doesn't exist in Windows, but what I CAN say is my problems were NOT fixed by using the "official" builds in Maven Central. My environment would only work correctly if I built the code manually using Gradle and the above fix proposed by @Enlade

@benbarkay benbarkay added bug Something isn't working verified and removed unverified labels Oct 13, 2018
@benbarkay
Copy link
Contributor

I've found this which is interesting:
https://stackoverflow.com/questions/34750664/annotations-containing-lambda-expressions-in-java-8

When I'm back home, I'll fiddle a little more. I suspect that if I refactor this comparator out of the annotation it'll all work as expected.

Which Java version do you run?

@joelkoz
Copy link

joelkoz commented Oct 13, 2018

@benbarkay - That sure sounds like the issue, though the bug report link says it was resolved in Oct 2016. The JRE on my Mac is build 1.8.0_91-b14 dated July 4 of this year.

Not sure of the date of the JRE on my Raspberry Pi, but the code also failed on that machine, and I built the image for that only 3 weeks ago, so I'm sure I have the latest JRE that is available. I suspect that if this does in fact pass the test on your machine, then its a bug in the JRE that still exists, and people are going to run into this issue one way or another. My vote would be to replace the code regardless, assuming the alternative code passes the test on your machine also.

@benbarkay
Copy link
Contributor

@Enlade is this still relevant in 1.0.12?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working verified
Projects
None yet
Development

No branches or pull requests

3 participants