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

Jump to bottom

Violation of one definition rule #424

Closed
paynterf opened this issue Jan 17, 2019 · 25 comments
Closed

Violation of one definition rule #424

paynterf opened this issue Jan 17, 2019 · 25 comments

Comments

@paynterf
Copy link
Contributor

When I compile my project using your wonderful i2cdevlib, I get the following output

Compiling debug version of 'FourWD_WallE2_V1' for 'Arduino/Genuino Mega w/ ATmega2560 (Mega 2560)' MPU6050.h: 401:7: warning: type 'struct MPU6050' violates one definition rule [-Wodr] class MPU6050 { MPU6050.h:401: note a different type is defined in another translation unit class MPU6050 { MPU6050.h:781: note the first difference of corresponding definitions is field dmpPacketBuffer uint8_t *dmpPacketBuffer MPU6050.h:983: note a field with different name is defined in another translation unit uint8_t devAddr

I know I can suppress these errors by adding '-Wodr' to my compiler switches, but this doesn't seem like a good idea. Any idea why this is happening?

VS2017 V15.9.5, Visual Micro V1.1811.24

TIA,

Frank
@jrowberg
Copy link
Owner

I haven't seen this one before. Is Visual Micro auto-generating any code, or otherwise mangling the original? The arrangement of the main "raw-only" implementation of MPU6050 code together with the DMP-capable add-on is far from ideal, but I don't believe there is any overlap or redefinition of variables, classes, structs, etc.

@paynterf
Copy link
Contributor Author

paynterf commented Jan 17, 2019 via email

@jrowberg
Copy link
Owner

I don't think struct MPU6050 appears anywhere in the original code, so if you can find that in any intermediate output, that file would be helpful to look over.

@paynterf
Copy link
Contributor Author

I found

typedef struct MPU6050_t { uint8_t devAddr; uint8_t buffer[14]; } MPU6050_t;

in C:\Users\Frank\Documents\Arduino\i2cdevlib\dsPIC30F\MPU6050\MPU6050.h

Which clued me in to the fact that I hadn't copied the appropriate files to my project directory, and the compiler was (I think) processing all the MPU6050.H files, not just the one I wanted - yikes!

However, I now have a completely new problem. When I compile with all relevant files in the project directory, I get

`Compiling 'MPU6050Test1' for 'Arduino/Genuino Mega w/ ATmega2560 (Mega 2560)'

ccIJMOF2.ltrans0.ltrans.o*: In function global constructors keyed to 65535_0_MPU6050Test1.cpp.o.2001
(.text.startup+0x74): undefined reference to MPU6050::MPU6050()

Error linking for board Arduino/Genuino Mega w/ ATmega2560 (Mega 2560)
Build failed for project 'MPU6050Test1'

ccIJMOF2.ltrans0.ltrans.o*: In function main
(.text.startup+0x1cc): undefined reference to MPU6050::initialize()
(.text.startup+0x1da): undefined reference to MPU6050::testConnection()
(.text.startup+0x25c): undefined reference to MPU6050::getMotion6(int*, int*, int*, int*, int*, int*)

collect2.exe*: error: ld returned 1 exit status`

And this is with the simplest possible example file - your 'C:\Users\Frank\Documents\Arduino\i2cdevlib\Arduino\MPU6050\examples\mpu6050_raw.ino' example, with no modifications. I have tried Build cleans, closing and re-opening VS2017, and even restarting my PC - nothing helps and I'm completely stymied. I have been working with i2cdev and the MPU6050 code for many months now with no problems, and all of a sudden everything is blowing up - help! ;-).

If it helps, I have uploaded a ZIP file containing my entire test project folder.

TIA,

Frank

MPU6050Test1.zip

@jrowberg
Copy link
Owner

Does the .ino example open and build cleanly in the Arduino IDE, as a starting point? I have not used Visual Micro in any meaningful way, so I'm not sure what it might be doing to complicate the process. Knowing that the correct code in the correct place compiles in the original IDE will be a good starting point.

@paynterf
Copy link
Contributor Author

paynterf commented Jan 18, 2019 via email

@jrowberg
Copy link
Owner

By the way, if you're not already aware, just for fun you might be interested in looking at a fledgling clean-slate attempt at fixing all of the little (and not so little) quirks about I2Cdevlib:

https://community.perilib.io/t/perilib-i2cdevlib-reborn/15

I2Cdevlib will stick around for sure, but I'm working slowly towards a better end result in the only way I think is possible, given the history, community, and complexity now tightly woven into the original project.

@paynterf
Copy link
Contributor Author

It compiles in the Arduino IDE if I first remove SBWire.h/cpp from the project folder. Apparently the Arduino IDE compiles everything in the sketch folder, whether or not it is referenced in the main program

@paynterf
Copy link
Contributor Author

paynterf commented Jan 26, 2019 via email

@paynterf
Copy link
Contributor Author

Jeff,
I think I found the reason I was getting 'One definition' violation warnings. Apparently this started happening when the board platform manager for the AVR boards went from 1.6.21 to 1.6.22 - see this post

https://forum.arduino.cc/index.php?topic=593211.0

Frank

@jrowberg
Copy link
Owner

Hi Frank,

Maybe I'm missing it, but are you sure you attached the verbose compiler output? I don't see it in your reply from yesterday afternoon.

@paynterf
Copy link
Contributor Author

paynterf commented Jan 26, 2019 via email

@paynterf
Copy link
Contributor Author

paynterf commented Jan 28, 2019 via email

@jrowberg
Copy link
Owner

Out of curiosity, what happens if you temporarily move the RTIMU library (in whatever form(s) you have it) entirely out of the library/include path used by the IDE, so that it can't be compiled in? Is it possible that there is some MPU6050 name overlap from something in that code, and the IDE is automatically pulling it in because of some auto-include feature. Or something. I'm grasping at straws here, but it's an idea.

@paynterf
Copy link
Contributor Author

paynterf commented Jan 29, 2019 via email

@paynterf
Copy link
Contributor Author

paynterf commented Jan 30, 2019 via email

@paynterf
Copy link
Contributor Author

paynterf commented Feb 1, 2019 via email

@jrowberg
Copy link
Owner

jrowberg commented Feb 1, 2019

Hi Frank,

It seems the attached files are still getting stripped out of your email replies to Github issues (or at least this one). Can you post them in a comment via the web interface?

@paynterf
Copy link
Contributor Author

paynterf commented Feb 1, 2019

Jeff,

Here they are. They might be getting stripped out by my email provider (Google Gmail), which I now recall doesn't like ZIP files :-(

MPU6050.zip

@paynterf
Copy link
Contributor Author

paynterf commented Feb 4, 2019

Jeff,

OK, I finally have it all figured out - I think. The 'one definition rule' (ODR) warning is caused when the compiler/linker sees code that can produce two different definitions for the same object. If that can happen, EVER, then an ODR violation warning is issued. Believe it or not, that is exactly what happens when the compiler processes MPU6050.H - it sees that there are some conditions for which two different descriptions of the MPU6050 class could exist - and says "no no". The relevant portion of the class definition is shown below:

  private:
        uint8_t devAddr;
        uint8_t buffer[14];
    #if defined(MPU6050_INCLUDE_DMP_MOTIONAPPS20) or defined(MPU6050_INCLUDE_DMP_MOTIONAPPS41)
        uint8_t *dmpPacketBuffer;
        uint16_t dmpPacketSize;
    #endif

When the compiler sees these lines, it says to itself; "Hmm, the way this is written, it is theoretically possible for there to exist two different versions of 'Class MPU6050' one with just two private member variables (devAddr & buffer) and one with four (with the addition of dmpPacketBuffer & dmpPacketSize), and this is a strict no-no; I'm going to whack that programmer across the head with an ODR violation!"

If the #ifdefined and #endif lines are commented out - the ODR warning goes away

Now, I suspect nobody has ever had a problem with this issue, as it would be very unlikely to have a project where BOTH versions of MPU6050 are in play, but of course the compiler doesn't see it this way. To address this problem, I think you might want to create a derived class (maybe MPU6050_DMP?) that simply adds the two additional private variables, or maybe better yet - simply remove the #ifdefined/#endif and let the two additional private variables tag along in every MPU6050 object, whether or not it is needed. If not, users will continue to see the ODR warning.

On a slightly different, but related subject, the OTHER warning was due to a potential integer overrun in the dmpGetGravity() function, as shown below:

uint8_t MPU6050::dmpGetGravity(int16_t *data, const uint8_t* packet) {
    /* +1g corresponds to +8192, sensitivity is 2g. */
    int16_t qI[4];
    uint8_t status = dmpGetQuaternion(qI, packet);
    data[0] = ((int32_t)qI[1] * qI[3] - (int32_t)qI[0] * qI[2]) / 16384;
    data[1] = ((int32_t)qI[0] * qI[1] + (int32_t)qI[2] * qI[3]) / 16384;
    data[2] = ((int32_t)qI[0] * qI[0] - (int32_t)qI[1] * qI[1]
	       - (int32_t)qI[2] * qI[2] + (int32_t)qI[3] * qI[3]) / (2 * 16384);
    return status;
}

if the last line of the above calculation is changed to (note the addition of 'UL')
- (int32_t)qI[2] * qI[2] + (int32_t)qI[3] * qI[3]) / (2 * 16384UL);
then this warning goes away as well.

Regards,

Frank

@jrowberg jrowberg mentioned this issue Feb 27, 2019
Closed
@paynterf paynterf mentioned this issue Apr 14, 2019
Closed
@Massyp
Copy link

Massyp commented Jun 12, 2019

Hello every body,

I m trying to do what you did to eliminate the violations warning, but I don't don't understand how you did it. Because, I read the two library programs, and I saw that if we comment the two line as you said, the other program will not be able to use the methods defined under this program. And it display me lot of errors,
So, if you can share us the final solution it will be helpful

Thank you at advance

@paynterf
Copy link
Contributor Author

Massyp,

Hmm, not sure what the problem is: If you change

private:
uint8_t devAddr;
uint8_t buffer[14];
#if defined(MPU6050_INCLUDE_DMP_MOTIONAPPS20) or defined(MPU6050_INCLUDE_DMP_MOTIONAPPS41)
uint8_t *dmpPacketBuffer;
uint16_t dmpPacketSize;
#endif

to
private:
uint8_t devAddr;
uint8_t buffer[14];
uint8_t *dmpPacketBuffer;
uint16_t dmpPacketSize;

Eliminating the #if defined... and #endif lines, you should be good to go.

Does this help?

Frank

@Massyp
Copy link

Massyp commented Jun 13, 2019

Hi Frank,
Are you talking about the code in this link :

https://github.com/jrowberg/i2cdevlib/blob/master/Arduino/MPU6050/MPU6050.h

If not, I think that I'm using the wrong one !!

Thank you for responding

@Massyp
Copy link

Massyp commented Jun 13, 2019 •
edited

Hi frank,

I think that I used an an old version of this code : in the MPU6050.h code, they declare " uint8_t *dmpPacketBuffer;
uint16_t dmpPacketSize;" as a public variables inside the special methods for MotionApps 2.0 implementation and inside the special methods for MotionApps 4.1 implementation

Now I resolve it inspiring from your advice just by cutting these two lines and declare it as a private variables as you said

Thank you Frank

@paynterf
Copy link
Contributor Author

Massyp,

I am just repaying a VERY small part of the debt I owe to all the others who have helped me over the years. You can thank me by helping someone else sometime in the future ;-).

Frank

@GitMoDu GitMoDu mentioned this issue Apr 27, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jrowberg @paynterf @Massyp