leo25 / arduino

Automatically exported from code.google.com/p/arduino
0 stars 0 forks source link

Stream-based Wire library doesn't accept zero #527

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Wire.write(0x00);

gives the following error:

 error: call of overloaded 'write(int)' is ambiguous
Wire.h:55: note: candidates are: virtual void TwoWire::write(uint8_t)
Wire.h:56: note:                 virtual void TwoWire::write(const char*)

I solved it with an ugly hack, like so:

byte zero = 0x00;
Wire.write(zero);

A better solution would be nice.

Original issue reported on code.google.com by tom.i...@gmail.com on 22 Apr 2011 at 8:01

GoogleCodeExporter commented 9 years ago
you can cast it to remove the ambiguity

Wire.write((uint8_t) 0x00);

No ?

Original comment by FRap...@gmail.com on 25 Apr 2011 at 10:52

GoogleCodeExporter commented 9 years ago
How about just adding these lines in Print.h?

    inline void write(unsigned long n) {write((uint8_t)n);}
    inline void write(long n) {write((uint8_t)n);}
    inline void write(unsigned int n) {write((uint8_t)n);}
    inline void write(int n) {write((uint8_t)n);}

These give the compiler the help it needs to "know" int (and the other 3) 
should use the uint8_t function.

Original comment by paul.sto...@gmail.com on 7 May 2011 at 11:16

GoogleCodeExporter commented 9 years ago
It looks like in 1.0, you have to replace "void" with "size_t". My code that 
includes Wire.h works when I include it within the two "virtual"s:

    virtual size_t write(uint8_t) = 0;
    inline size_t write(unsigned long n) {write((uint8_t)n);}
    inline size_t write(long n) {write((uint8_t)n);}
    inline size_t write(unsigned int n) {write((uint8_t)n);}
    inline size_t write(int n) {write((uint8_t)n);}
    size_t write(const char *str) { return write((const uint8_t *)str, strlen(str)); }
    virtual size_t write(const uint8_t *buffer, size_t size);

Original comment by andrewwi...@gmail.com on 17 Sep 2011 at 9:27

GoogleCodeExporter commented 9 years ago
Adding the following lines to Class Print in Print.h fixed the issue:

    inline size_t write(unsigned long n) {write((uint8_t)n);}
    inline size_t write(long n) {write((uint8_t)n);}
    inline size_t write(unsigned int n) {write((uint8_t)n);}
    inline size_t write(int n) {write((uint8_t)n);}

Original comment by fabio.va...@gmail.com on 30 Nov 2011 at 4:05

GoogleCodeExporter commented 9 years ago
When I tested this before, putting them Wire.h (and any other derived classes) 
produced more efficient code than putten them in Print.h (the base class).

Original comment by paul.sto...@gmail.com on 30 Nov 2011 at 6:36

GoogleCodeExporter commented 9 years ago
Since write now returns a size_t, those includes should be:

    inline size_t write(unsigned long n) { return write((uint8_t)n); }
    inline size_t write(long n) { return write((uint8_t)n); }
    inline size_t write(unsigned int n) { return write((uint8_t)n); }
    inline size_t write(int n) { return write((uint8_t)n); }

I'll attach a patch, which just adds these 4 linse to Wire.h

Original comment by paul.sto...@gmail.com on 30 Nov 2011 at 9:15

Attachments:

GoogleCodeExporter commented 9 years ago
Looking good here.

Original comment by fabio.va...@gmail.com on 1 Dec 2011 at 8:24

GoogleCodeExporter commented 9 years ago
https://github.com/arduino/Arduino/commit/6a6ed3d10ad1470283d7771906ce81ad97fa06
f0

Although if this comes up for lots of other classes that inherit from Print, 
maybe we should move these overloads there?

Original comment by dmel...@gmail.com on 2 Dec 2011 at 9:45

GoogleCodeExporter commented 9 years ago
Issue 743 has been merged into this issue.

Original comment by dmel...@gmail.com on 7 Dec 2011 at 11:29

GoogleCodeExporter commented 9 years ago
This change introduces the possibility of hard-to-detect bugs: attempting to 
write 2+ byte values via a single-argument form of Wire.write(...). With this 
change, the compiler will accept such code without complaint, and yet the 
non-lowest bytes will be silently dropped. Debugging this could become a 
nightmare: imagine if at first, the 2-byte integer you're sending over the wire 
is between 0 and 255. You test this and [for some reason] it works well. Then, 
sometime later, the value goes negative or becomes larger than 255. It is 
likely that the only way to detect such a change is to put a logic analyzer on 
the TWI bus, which probably means that the user has already spent some time 
looking for software errors on his/her side.

I understand that most people will be acquainted to TWI's single-byte-at-a-time 
nature. There's also Wire.Write(uint8_t * data, size_t quantity). Perhaps the 
person most likely to get things wrong is someone new to TWI. He or she would 
see an overload that accepts an int, and decide to use it, assuming it exists 
because an int is a valid datatype. Or, he or she simply writes the code and 
since it compiles, surely at least the _datatype_ is correct? Arduino is 
supposed to be friendly to beginners, but this change does exactly the 
opposite! Note that searching for "call of overloaded 'write(int)' is 
ambiguous" quickly brings one to a solution to the Wire.write(0) error message.

I suggest this change be reverted and that we go back to using C's type system 
as best as we can. C does not give very many compile-time guarantees; let us 
use the ones we have!

Original comment by labre...@gmail.com on 31 May 2012 at 10:42

GoogleCodeExporter commented 9 years ago
I think it's important that Wire.write(0) works without casting, but I'd be 
open to another solution for that.

Original comment by dmel...@gmail.com on 1 Jun 2012 at 1:04

GoogleCodeExporter commented 9 years ago
I did some more exploration of the issue and found out that one would need the 
-Wconversion compiler flag in order to catch integer narrowing. It turns out 
that the Arduino folks, far from using something like -Wall, use -w, which 
suppresses all warnings. So, unless they are willing to take a very different 
avenue than they do now (such as have an option to specify warning reporting 
level), the issue I raised is lost in the "noise".

Original comment by labre...@gmail.com on 6 Jun 2012 at 7:10