Go Down

Topic: Will Arduino team ever do better than this?! (Read 12901 times) previous topic - next topic

liuzengqiang

So there are many of us using Arduino libraries. What should we do? I remember that I kept reading their code just to understand what might happen in certain situations, even begin() return values were not mentioned in docs, but why not? Say, if I want to improve the stability of arduino wifi library, what can I can? Form a study group to first understand and comment the library? I'm thinking about that. Arduino core code is poorly commented, so giving me an impression that they never thought about community involvement in their core code, which may be a wrong impression but it surely works against anyone wanting to understand the core code. What to do? Write a book?

I get the commercial counterpart not performing to their expectation part. I'm using digi's xbee wifi modules. Their firmware sucked and still sucks if you want to use it as a server. The internal mechanism (FW) intercepts requests and then does nothing, leaving developers with empty hands.

I don't know where that UNO not open source came from. The poster may be confused with GNU GPL lesser license terms. It only opens the source and won't demand derived work to be open sources, which is what I think is great.
Serial LCD keypad panel,phi_prompt user interface library,SDI-12 USB Adapter

Robin2


Arduino core code is poorly commented,


I've just picked this quote as a crude summary of your entire post.

I agree. But point me to some Open Source software that is considerably better documented?

I have a little familiarity with Ruby on Rails which is widely used for professional websites and it has little or no documentation beyond a few examples and some documentation that is automatically extracted from the code.

Poor documentation is a widespread problem for Open Source software. And it's easy to see why. People like writing code and they don't like writing documentation. Microsoft, Oracle etc pay people to write documentation. Writing documentation would probably take as much time as writing the code.

...R
Two or three hours spent thinking and reading documentation solves most programming problems.

liuzengqiang

Robin,

Sometimes it takes a good programmer to a lot of time to write good code. Always, it takes a good programmer a lot of time to understand the good code that didn't come with comments. Mind you, these two good programmers might be the same programmer, one in the past that didn't write comments and one in the future that forgot how the past person wrote it. I taught myself coding at a young age and didn't like to write comments much. Why do the obvious and waste time?! I understand everything I wrote!!! Oh, I even remember what I put in my CX register just before a bunch of short calls and I can use it at a later time as long as I don't use it for something else, no need to comment on that or push CX. As I age, I more and more rely on comments that I wrote, especially for longer projects and libraries I released. Even for my own libraries, I can't just jump in and do a mod or fix if you pay me to. I need time to relearn what I wrote. I use doxygen for some of my libraries. Yes, it's a lot of writing. But if you don't write it as an author, who is going to write it for you? Paying someone else to do your house keeping is OK as long as you are making money on the code. I don't make money on the libraries. Well, I got about $40 donations for the entire time I have them online.

BTW, you've probably not seen my code but I'm not like many programmers. I'm a technical writer.
Serial LCD keypad panel,phi_prompt user interface library,SDI-12 USB Adapter

westfw

Quote
So there are many of us using Arduino libraries...

When you say "Arduino Libraries", do you mean the official libraries that come with the IDE, or are you including the various third-party libraries?  Your examples have been about the official libraries, and they should be "better."  Asking random parties that have made their libraries available because "it seems to work and I thought someone else might find it useful" to do real maintenance is probably not possible.

Quote
Say, if I want to improve the stability of arduino wifi library, what can I do?

1) Get your own github repository for just the WiFi libraray, start calling attention to its bugs and weaknesses, and start adding code.  When you get to the point where it is significantly better than the arduino repository, tell them "hey, our code is working better than yours, has a bunch of people using it, and has better documentation.  Don't you think you should merge it into the official distribution?"   If you're talking about "community support", a more isolated bit of code is likely to be easier to get people involved with than the core libraries that are buried somewhere in the IDE source tree.
2) Jump ship for a different WiFi provider ("Vote with your wallet.")  There seem to be a lot of (TI) CC3000 boards and modules popping up all over (mostly using the Adafruit Arduino library.)    And it seems (at the moment, anyway) to have more aggressive support (from Adafruit, from the Community, and from TI.  Not that "oh, you need to upgrade your firmware before you can use the new library" or "we're waiting for TI to fix that in the proprietary party of their code" are great places to be, either. ) And there are SO MANY WiFi modules out there (not all made into Arduino shields.)  Shake any trade show and a dozen or so thumbnail-sized WiFi modules fall out, using all sorts of different chipsets and different proprietary software.  (Because "Internet of Things"!!!!)  The ones that look like the company is two guys in the spare bedroom aren't half as scary as the ones that think that Wireless for IoT should be some sort of subscription-based cloud service (like "Exosite", who used to be prominent in TI's "Connected Launchpad" collateral (but I don't see them so much any more...)  "Spark Core" is another.)
3) OTOH, there's also the wireless card firmware.  At least some of that is accessible on the Arduino WiFi card (even though you'll need to become an AVR32 programmer to modify it.)  On most modules, the firmware is fully proprietary.
4) You might be surprised how little you need to figure out about the current code before you can make improvements.  (of course, that's not a great way to end up with a beautiful library, but...  ("Rough consensus and running code" - a motto that built the internet. )


liuzengqiang

I was referring to official libraries. If you open the ethernet library, there is exactly one line of comment in EthernetServer.cpp The UPD code has more comments to help reading, maybe from a different author or period of time. I mentioned my only contributed library as a counter example. Every few lines you will find a comment. It's supposed to help with reading the code. Even stating the obvious is helpful to those just started understand the code.

If you look at SoftwareSerial, you see what I'm talking about (lines of comments), because someone other than the core team wrote it. Same with sdfile.cpp, which is wrapped around by the SD class. Tons of comments. Then you go back to Esplora, wifi etc. that are core lib from start, only some under utilities have comments but their wrappers don't have much. I must be holding a very high standard. I thought open source is for others to build upon and learn from you. What use is a bunch of answers without derivation? "Got to learn by just using it but not questioning the source!" That sounds familiar.

I will see what I can do. I'm not an AVR32 programmer, not interested, not in my field. I'll start with some simpler core library and simply see how to get github to work and make some comments on simpler libraries.

Yes, I get that "latest library works with latest wifi firmware" joke. Will they ever release an exact version number instead of using "latest" for everything? I will have to wait and see. The reason I'm not using their latest everything is that I know my IDE 1.0 works. If I migrate to 1.0.5, I will have to test out everything and it takes weeks to test all functions including telemetry and we may have to go back, cause all units deployed in the field have whatever official wifi firmware stuck there. I don't expect a technical person to do that update. It's complicated (2 firmware updates and hardly an indicator). I'm nervous myself. Did it a few times before I got it to work and no way to go back to the original firmware (cause no version number). High level of frustration all the time.

Digi has xbee everything. Their xbee wifi FW is far from ideal but at least they have version number and when new things didn't work, I went back one FW and it worked. Will wait on their new FW but this absolutely depends on the demand on the modules, like real businesses do.
Serial LCD keypad panel,phi_prompt user interface library,SDI-12 USB Adapter

westfw

Quote
If you open the ethernet library, there is exactly one line of comment in EthernetServer.cpp

Yes, that's pretty bad.  Especially since accept() really needs some documentation; pretty confusing stuff.

On top of that, there's this snip from the documentation page for the Ethernet library:
Code: [Select]
void loop() {
  // if an incoming client connects, there will be bytes available to read:
  EthernetClient client = server.available();
  if (client == true) {

That's just EVIL, isn't it?  "client" would traditionally be null (which happens to be the same as false) or an object address; it would never be "true."  (I see that EthernetClient.cpp overrides "operator bool" to allow this.   I guess that works (?  How does it tell that this a boolean and not a numeric comarison?), but I don't like it!

pYro_65

#21
Jun 27, 2014, 05:00 am Last Edit: Jun 27, 2014, 05:13 am by pYro_65 Reason: 1

That's just EVIL, isn't it?  "client" would traditionally be null (which happens to be the same as false) or an object address; it would never be "true."  (I see that EthernetClient.cpp overrides "operator bool" to allow this.   I guess that works (?  How does it tell that this a boolean and not a numeric comarison?), but I don't like it!


It may be confusing, but useful. Seeing as were all in the mood for nice long posts,

The standard explains statements in an 'if' like this: " The value of a condition that is an expression is the value of the
expression, contextually converted to bool
"

As the class provides the conversion ( C++ does not know how to do it ), it can be used.
The compiler will always call a conversion operator on anything other than objects of the same type, primitives simply use the global versions.

But, in the example you provided, the '== true' causes the conversion to happen before the 'if' rules take place. And that is just a standard C-style cast.  It can also be written as simply: if( client ){ and the if statement directly calls the operator bool()

Overloading the conversion operator may seem strange, but if you think of it in the same way C++ uses it internally, it makes sense,
If you want to extend a platform with say an int24_t and uint24_t type, by adding the conversion operators to the standard ints, it could be used seamlessly in expressions mixing different (standard) integer types.

The standard provides this text:
Quote
7 An id-expression is a restricted form of a primary-expression. [Note: an id-expression can appear after . and -> operators (5.2.5). --end note] 8 An identi?er is an id-expression provided it has been suitably declared (Clause 7). [Note: for operator- function-ids, see13.5; for conversion-function-ids, see 12.3.2


The section mentioning conversion-function-ids is below.

Quote from: 12.3.2 Conversion functions (class.conv.fct)

1 A member function of a class X having no parameters with a name of the form
conversion-function-id:
operator conversion-type-id
conversion-type-id:
type-speci?er-seq conversion-declarator opt
conversion-declarator:
ptr-operator conversion-declarator opt

speci?es a conversion from X to the type speci?ed by the conversion-type-id. Such functions are called
conversion functions. No return type can be speci?ed. If a conversion function is a member function, the
type of the conversion function (8.3.5) is "function taking no parameter returning conversion-type-id". A
conversion function is never used to convert a (possibly cv-quali?ed) object to the (possibly cv-quali?ed)
same object type (or a reference to it), to a (possibly cv-quali?ed) base class of that type (or a reference to
it), or to (possibly cv-quali?ed) void.116
[Example:
struct X {
operator int();
};
void f(X a) {
int i = int(a);
i = (int)a;
i = a;
}
In all three cases the value assigned will be converted by X::operator int(). -- end example ]


Side info: Where it states 'never convert to same type" it  indicates you need to use a copy constructor in X, as X should be able to handle itself.

Maybe the Arduino team could define another variable or define to hide the obscure conversion.
Code: [Select]

const bool CLIENT_READY = true;
//...
if( client == CLIENT_READY ){


A  ready() or active() method may be better for this situation, no hidden conversions. The client class does however provide available() which does the same check as the operator bool, however it also does a little extra work. You could possibly reduce flash size a pinch by only using available(). But you may need to build the loop differently, as 0 is returned on no data, not just a bad port number, still useful though.
Forum Mod anyone?
https://arduino.land/Moduino/

westfw

So code like:
Code: [Select]
EthernetClient::operator bool() {
  return _sock != MAX_SOCK_NUM;
}

invokes the function (and returns the bool value) whenever an EthernetClient object is referenced with no other qualifiers?

Note that "true" is not defined as a bool either, so I would have expected (as a C programmer) "client == true" to devolved to a comparison between a pointer and an int.

pYro_65

Quote
Note that "true" is not defined as a bool either, so I would have expected (as a C programmer) "client == true" to devolved to a comparison between a pointer and an int.


That may be true in the C world ( I really have no idea ), however C++ has defined true and false as bool, but only false has an explicit value.

Quote
2.14.6 Boolean literals [lex.bool]
boolean-literal:
false
true
1 The Boolean literals are the keywords false and true. Such literals are prvalues and have type bool.


But you are right, it is an integer comparison, bools are integers too:
Quote
3.9.1 Fundamental types [basic.fundamental]
6 Values of type bool are either true or false.
47 [ Note: There are no signed, unsigned, short, or long
bool types or values. -- end note ] Values of type bool participate in integral promotions (4.5).
7 Types bool, char, char16_t, char32_t, wchar_t, and the signed and unsigned integer types are collectively
called integral types.48 A synonym for integral type is integer type


But with regards to why C++ chooses to do a boolean comparison is simply to do with 'overload resolution' It will look for the most compatible conversion, if none is found or an ambiguous result occurs then an error is generated.

However, that code you posted no longer works, 7 months ago on git hub, there where changes to the Ethernet library.

They added two operators '==' & '!=' which effectively broke your code example, if its still in the docs we might have to annoy someone.
You now can only use operator bool in an explicit boolean expression.

Code: [Select]

if( client ){
//...
while( client ){


Using client == true now causes the compiler to use the overload for the ==, rather than a global comparison. These operators operate left to right, so == is in the context of client.

One great way to show how the operators are selected is to switch them around.

Code: [Select]
if( true == client ){

Now the opreator == is in the bool context and the comparison works fine, even with the EthernetClient mods.
Forum Mod anyone?
https://arduino.land/Moduino/

westfw

In this case, there is a "#define true 0x1" in Arduino.h, so C++ never gets to see it's version of "true", right?

pYro_65

#25
Jun 28, 2014, 10:14 am Last Edit: Jun 28, 2014, 10:17 am by pYro_65 Reason: 1

In this case, there is a "#define true 0x1" in Arduino.h, so C++ never gets to see it's version of "true", right?



Wowzers, that is a blunder and a half, they are trying to change the behavior of a bool.

True and false do not matter to much as a bool is promoted to an int of 1 or 0 before it can be compared. As a bool can be implicitly converted to an integer, the compiler is still happy to use the bool operator overload.

I wonder if 'they' realize that code like my example is very different in its execution. I always expected the Arduino boolean to act like a bool :

All these statements apart from one should return "true"
Code: [Select]

void setup() {
 Serial.begin(9600);
 Serial.println( true == (bool) 57 ? "true" : "false" );
 Serial.println( true == (boolean)57 ? "true" : "false" );
 Serial.println( true == bool( true ) ? "true" : "false" );
 Serial.println( bool( 1 | 2 | 4 | 8 ) == boolean( 1 | 2 | 4 | 8 ) ? "true" : "false" );
 Serial.println( true == false + 1 ? "true" : "false" );
 Serial.println( true + true == true ? "true" : "false" );
 Serial.println( bool( true + true ) == true ? "true" : "false" );
 Serial.println( boolean( true + true ) == true ? "true" : "false" );
}

void loop(){}


Output:
Quote
true
false
true
false
true
false
true
false


I then commented out the true, false defines, and the typedef for boolean.
Then I added in #define boolean uint8_t << this is so C programs can use a bool(ean)
Then in the sketch I added to the top:
Code: [Select]

#include "Arduino.h"
#undef boolean
#define boolean bool


And the results are below when using the standard true/false and boolean as an alias for bool not uint8_t:
Quote
true
true
true
true
true
false
true
true


Arduino can produce incorrect logic (not C++ logic), if you assume like the (Arduino) reference says:

"A boolean holds one of two values, true or false." and "Any integer which is non-zero is true ( in a boolean sense )."

As I have clearly shown that:  "true != (boolean)57" their quotes above are only true for the real standard 'bool'.

Their specific Arduino conventions truly are defining a new language. Maybe liudr is onto something here, stuff like this definitely needs proper documentation.

Note: the standard explicitly disallows #defining keywords:

Quote
17.4.3.1.1 Macro names [lib.macro.names]
... Nor shall such a translation unit define macros for names lexically identical to keywords.
Forum Mod anyone?
https://arduino.land/Moduino/

westfw

What should a C function (extern "C" {}) return for "true" ?  Is there a standard way to define true in a head such that it gets #defined for C code, but not for C++ code?

pYro_65

#27
Jun 28, 2014, 12:17 pm Last Edit: Jun 28, 2014, 12:21 pm by pYro_65 Reason: 1
They could use two defines TRUE and FALSE, or just undef them before the sketch sees them. However the defines do not seem to produce differing results compared to the standard values. Every where I have tested a the real values, they are promoted to an int of 0 or 1 before an expression is evaluated as opposed to an int down casting to a bool.

The problem is more in the fact that boolean is a byte, which does not conform to the rule of a bool  "A boolean holds one of two values, true or false."

It really needs to be a real bool when used in a C++ context.

I posted an issue on the Arduino github too: https://github.com/arduino/Arduino/issues/2147
Forum Mod anyone?
https://arduino.land/Moduino/

kowalski

There is a standard definition for this. See http://pubs.opengroup.org/onlinepubs/009695399/basedefs/stdbool.h.html and  http://clang.llvm.org/doxygen/stdbool_8h_source.html. This could be added to AVR GCC or intermediately to Arduino.

Cheers!

robtillaart

Good and interesting analysis Pyro!

Still imho mixing datatypes (int and bool(ean)) is always tricky.
With comparing integer with booleans we loose "significant truth" bits.

similar as loosing significant digits when converting long to float and or vice versa.

So in the end the programmer is responsible to know the limits of a language
(see book safer C from Les Hatton)
Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Go Up