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

Compilation fails with recent Wire.h changes #5287

Closed
6 tasks done
mcspr opened this issue Oct 28, 2018 · 4 comments
Closed
6 tasks done

Compilation fails with recent Wire.h changes #5287

mcspr opened this issue Oct 28, 2018 · 4 comments

Comments

@mcspr
Copy link
Collaborator

mcspr commented Oct 28, 2018

Basic Infos

  • This issue complies with the issue POLICY doc.
  • I have read the documentation at readthedocs and the issue is not addressed there.
  • I have tested that the issue is present in current master branch (aka latest git).
  • I have searched the issue tracker for a similar issue.
  • If there is a stack dump, I have decoded it.
  • I have filled out all fields below.

Platform

  • Hardware: Any
  • Core Version: cb05b86
  • Development Env: Arduino IDE / Makefile
  • Operating System: Linux

Settings in IDE

  • Module: Generic ESP8266 Module
  • Flash Mode: dio
  • Flash Size: 4MB
  • lwip Variant: v2 Lower Memory
  • Reset Method: nodemcu
  • Flash Frequency: 40Mhz
  • CPU Frequency: 80Mhz
  • Upload Using: SERIAL
  • Upload Speed: 115200

Problem Description

Commit bfcbd71 from #5226 changed twi_attachSlaveRxEvent argument signature from ..., int to ..., size_t and now when Wire.h is included compilation fails.

Looking at arduino-avr variant, cores/esp8266/twi.h and cores/esp866/core_esp8266_si2c.c seems to be in the wrong here and 'numBytes' should be int instead of size_t

void TwoWire::onReceiveService(uint8_t* inBytes, int numBytes)

void twi_attachSlaveRxEvent( void (*)(uint8_t*, size_t) );

twi_attachSlaveRxEvent(onReceiveService);

MCVE Sketch

#include <Arduino.h>
#include <Wire.h> // <---

void setup() {}

void loop() {}

Debug Messages

/home/builder/arduino-1.8.7/hardware/esp8266com/esp8266/libraries/Wire/Wire.cpp: In member function 'void TwoWire::begin(uint8_t)':
/home/builder/arduino-1.8.7/hardware/esp8266com/esp8266/libraries/Wire/Wire.cpp:84:42: error: invalid conversion from 'void (*)(uint8_t*, int) {aka void (*)(unsigned char*, int)}' to 'void (*)(uint8_t*, size_t) {aka void (*)(unsigned char*, unsigned int)}' [-fpermissive]
   twi_attachSlaveRxEvent(onReceiveService);
                                          ^
In file included from /home/builder/arduino-1.8.7/hardware/esp8266com/esp8266/libraries/Wire/Wire.cpp:31:0:
/home/builder/arduino-1.8.7/hardware/esp8266com/esp8266/cores/esp8266/twi.h:51:6: error:   initializing argument 1 of 'void twi_attachSlaveRxEvent(void (*)(uint8_t*, size_t))' [-fpermissive]
 void twi_attachSlaveRxEvent( void (*)(uint8_t*, size_t) );
      ^
===info ||| Using library {0} at version {1} in folder: {2} {3} ||| [Wire 1.0 %2Fhome%2Fbuilder%2Farduino-1.8.7%2Fhardware%2Fesp8266com%2Fesp8266%2Flibraries%2FWire ]
exit status 1
@mcspr
Copy link
Collaborator Author

mcspr commented Oct 28, 2018

Also mentioned in cb05b86#comments, but suggested change is other way around - fix other args to size_t

reaper7 referenced this issue Oct 28, 2018
* I2C slave support; resolving conflicts against current master

* removed unused argument, updateded to hopefully pass Travis

* cleaning up commit as requested by #5162 (review)

* cleaning up commit as requested by #5162 (review)

* type fix
@ascillato
Copy link
Contributor

Hi,

I can confirm that the core is not compiling if using the core from cb05b86 and this wire library in a sketch. So, for example, Sonoff-Tasmota will not compile either under this commit.

Also I can confirm that the fix proposed in the comments don't work either. The Arduino core stills not compile. Sorry.

Please, if there is something else I can help, do not hesitate on asking.

@devyte
Copy link
Collaborator

devyte commented Oct 28, 2018

The correct arg type is in fact size_t, and not int, because the value can't be negative. At the very least, it needs to be unsigned.
The proposed fix is in #5289 , and it compiles for me. If the core doesn't compile for you, I'll need further details.

@devyte devyte self-assigned this Oct 28, 2018
@devyte devyte added this to the 2.5.0 milestone Oct 28, 2018
@ascillato
Copy link
Contributor

Hi,

I can confirm that with the PR #5289, now the core compiles fine.

Please, merge it.

Thanks a lot for your work 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants