Fix bug warning compilation

[alambrec]

Bug warning during compilation with PlatformIO in Atom.

\\Before
int32_t lookupTimeout = millis()+ MESH_LOOKUP_TIMEOUT;
\\After
uint32_t lookupTimeout = millis()+ MESH_LOOKUP_TIMEOUT;

\\Before
RF24NetworkHeader& header = *(RF24NetworkHeader*)network.frame_buffer;
\\After
RF24NetworkHeader header;
memcpy(&header, network.frame_buffer, sizeof(RF24NetworkHeader));

[Avamander]

What's the warning?

[alambrec]

Sorry, I forgot the warning messages :

.piolibdeps/RF24Mesh_ID209/RF24Mesh.cpp: In member function 'uint8_t RF24Mesh::update()':
.piolibdeps/RF24Mesh_ID209/RF24Mesh.cpp:58:61: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstri
ct-aliasing]
RF24NetworkHeader& header = *(RF24NetworkHeader*)network.frame_buffer;
^
.piolibdeps/RF24Mesh_ID209/RF24Mesh.cpp: In member function 'bool RF24Mesh::write(const void*, uint8_t, size_t, uint8_t)':
.piolibdeps/RF24Mesh_ID209/RF24Mesh.cpp:112:21: warning: comparison between signed and unsigned integer expressions [-Wsign-compar
e]
if(millis() > lookupTimeout || toNode == -2){
^

I test physically my fix between 2 nodes, and it's all ok ! It's not a big warning, but if we can to fix it, it's better !

sorry for my poor english !

Thanks 😄 ℹ️ I use Atom 1.12.9, PlatformIO 1.7.1 (Core 3.2.1), OSX 10.11.6

[Avamander]

Hmm, I should see if the function is as fast still. I am not convinced it compiles to the same result.

[alambrec]

Ok, i'm student so you are much better than me 👍 Physically it's work properly, with a node in "controller mode" and an other node in "node mode". I call in loop() the RF24Mesh::update() and nodes seems works ! If you found that this fix is not correct, I will be interested to know the problem with it ;) But I think you're right with the execution speed, which can be less fast than your code.

This fix is the result of StackOverflow. :link: http://stackoverflow.com/a/3246340

[mamama1]

hi

RF24NetworkHeader& header = (RF24NetworkHeader)network.frame_buffer;

this indirectly seems to cause a crash on my teensy 3.5 when trying to access the header data later. maybe memcopy is the cleaner solution. will try that out tomorrow.

regarding saving millis in a signed int32 and then even comparing with unsigned int32 i must say this looks messy to me. are there any drawbacks of using uint32_t instead?

[alambrec]

Hi mamama1,

Thx for your answer !

Resume of warning :

millis() > lookupTimeout

Regarding to your question, the unsigned int 32 is most appropriate (for me) because the return of function millis() is also an unsigned int 32. And like you only make sums with lookupTimeout, it's unnecessary to stock this value in signed int 32 because this variable is always positive !

I agree with you that it's a minor problem ! This is not important but in some cases the comparison is poorly done due to side effects. The range value of unsigned int 32is 0 to 4 294 967 295 whereas for signed int 32is -2 147 483 648 to 2 147 483 647. So, in the case where return value of millis() will be higher than 2 147 483 647, the comparaison won't work properly. But it's very likely that the Arduino/Raspberry will have restarted by that time.

So, I hope I'm clear with this explanation 😄.

[mamama1]

Hi

that's exactly what I meant. comparing uint and int is not good and will lead to an error eventually. i was just asking if there is any performance gain in using int instead of uint or if this was just a simple mistake.

[alambrec]

Re,

Ok, sorry ! For me, it's no performance difference between to use uint_32 or int_32. Perhaps in case of signed int is less efficient than unsigned int because during a comparaison, the processor must check the sign of number before to compare the value of variable. But i'm not sure !