Go Down

Topic: Code plagiarism (Read 1 time) previous topic - next topic

Pedro147

I won't bore you too much with this, but it is really starting to annoy me. I have spent a lot of time scouring the internet for LED cube code over the last year or so and I have lost count of the number of times that I have come across this particular form of code.

Code: [Select]
#include <avr/pgmspace.h> // allows use of PROGMEM to store patterns in flash
#define CUBESIZE 4
#define PLANESIZE CUBESIZE*CUBESIZE
#define PLANETIME 3333 // time each plane is displayed in us -> 100 Hz refresh
#define TIMECONST 20 // multiplies DisplayTime to get ms - why not =100?

// LED Pattern Table in PROGMEM - last column is display time in 100ms units
// TODO this could be a lot more compact but not with binary pattern representation
prog_uchar PROGMEM PatternTable[] = {
// blink on and off

B0001,B0000,B0000,B0000,B0001,B0000,B0000,B0000,B0000,B0000,B0000,B0000,B0000,B0000,B0000,B0000,10,
B0011,B0000,B0000,B0000,B0011,B0000,B0000,B0000,B0000,B0000,B0000,B0000,B0000,B0000,B0000,B0000,10,

// More data of the above form

int LEDPin[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15};
  int PlanePin[] = {16, 17, 18, 19};
 
  // initialization
  void setup()
  {
  int pin; // loop counter
  // set up LED pins as output (active HIGH)
  for (pin=0; pin<PLANESIZE; pin++) {
  pinMode( LEDPin[pin], OUTPUT );
  }
  // set up plane pins as outputs (active LOW)
  for (pin=0; pin<CUBESIZE; pin++) {
  pinMode( PlanePin[pin], OUTPUT );
  }
  }
 
  // display pattern in table until DisplayTime is zero (then repeat)
  void loop()
  {
  // declare variables
  byte PatternBuf[PLANESIZE]; // saves current pattern from PatternTable
  int PatternIdx;
  byte DisplayTime; // time*100ms to display pattern
  unsigned long EndTime;
  int plane; // loop counter for cube refresh
  int patbufidx; // indexes which byte from pattern buffer
  int ledrow; // counts LEDs in refresh loop
  int ledcol; // counts LEDs in refresh loop
  int ledpin; // counts LEDs in refresh loop
 
  // Initialize PatternIdx to beginning of pattern table
  PatternIdx = 0;
  // loop over entries in pattern table - while DisplayTime>0
  do {
  // read pattern from PROGMEM and save in array
  memcpy_P( PatternBuf, PatternTable+PatternIdx, PLANESIZE );
  PatternIdx += PLANESIZE;
  // read DisplayTime from PROGMEM and increment index
  DisplayTime = pgm_read_byte_near( PatternTable + PatternIdx++ );
  // compute EndTime from current time (ms) and DisplayTime
  EndTime = millis() + ((unsigned long) DisplayTime) * TIMECONST;
 
  // loop while DisplayTime>0 and current time < EndTime
  while ( millis() < EndTime ) {
  patbufidx = 0; // reset index counter to beginning of buffer
  // loop over planes
  for (plane=0; plane<CUBESIZE; plane++) {
  // turn previous plane off
  if (plane==0) {
  digitalWrite( PlanePin[CUBESIZE-1], HIGH );
  } else {
  digitalWrite( PlanePin[plane-1], HIGH );
  }
 
  // load current plane pattern data into ports
  ledpin = 0;
  for (ledrow=0; ledrow<CUBESIZE; ledrow++) {
  for (ledcol=0; ledcol<CUBESIZE; ledcol++) {
  digitalWrite( LEDPin[ledpin++], PatternBuf[patbufidx] & (1 << ledcol) );
  }
  patbufidx++;
  }
 
  // turn current plane on
  digitalWrite( PlanePin[plane], LOW );
  // delay PLANETIME us
  delayMicroseconds( PLANETIME );
  } // for plane
  } // while <EndTime
  } while (DisplayTime > 0); // read patterns until time=0 which signals end
  }



Nearly everybody who shows this code, or a close facsimile of, seems to think that they actually wrote it and doesn't seem to have the common decency to acknowledge the true author of this nice concise piece of coding. Hey look, I am flat out writing my own code of any level of complexity but I would not even think of trying to take credit for someone else's work. Doesn't this sort of thing go against the whole idea of open source and cooperative interaction between like minded individuals sharing a common interest? I will now step down from my soapbox, and thanks for letting me vent
http://www.pedroduino.com

Nick Gammon

It's probably done inadvertently. I try to credit good ideas I get from other people's code when I use them, but if I had to make major changes, or the code snippet was small, I may use it without crediting the author. And of course, from what you just said, I might be giving credit to the wrong author.
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

Pedro147

I see your point Nick, it can be difficult to remember where you first saw a piece of code and people of a limited skill level like me are always relying on dissecting other peoples code as a method of learning. After a few experimental modifications who knows where you got it from. That is why now I try to remember to paste a link at the start of any code that I come across so that I can acknowledge the true author and so that I can find the source again if I require further information. It is just that in the case of this particular code even the comments are exactly the same so it just makes me wonder. We are not talking snippets here but reams I am sure that no one would ever accuse you of this heinous crime  XD
http://www.pedroduino.com

robtillaart

TO find the author you could try to google a substantial snippet from it and take the last entry of the hits
Rob Tillaart

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

Pedro147

Yes Rob I did do that out of interest, and as I knew, it had been posted all over the place with numerous people claiming credit for it. It's no biggie but it just annoys me  :
http://www.pedroduino.com

Nick Gammon

Code: [Select]

// load current plane pattern data into ports
  ledpin = 0;
  for (ledrow=0; ledrow<CUBESIZE; ledrow++) {
  for (ledcol=0; ledcol<CUBESIZE; ledcol++) {
  digitalWrite( LEDPin[ledpin++], PatternBuf[patbufidx] & (1 << ledcol) );
  }
  patbufidx++;
  }
 
  // turn current plane on
  digitalWrite( PlanePin[plane], LOW );
  // delay PLANETIME us
  delayMicroseconds( PLANETIME );
  } // for plane
  } // while <EndTime
  } while (DisplayTime > 0); // read patterns until time=0 which signals end
  }


Well, it's not my code. The indentation is terrible. ;)
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

Coding Badly


This is the earliest dated reference I could find...
http://cratel.wichita.edu/blogs/engr101fall2010/2010/11/01/michael-leblanc-3/

Which includes this...
Based on ledcube.c from Make: September 7, 2007 weekend podcast
http://blog.makezine.com/archive/2007/09/make_a_pocket_led_cube_we.html

The Make source code does not include a license file.  ledcube.c includes this...
27-August-07
Distributed under Creative Commons 2.5 -- Attib & Share Alike

The Creative Commons license summary is this...
http://creativecommons.org/licenses/by-nc-sa/2.5/
Attribution -- You must attribute the work in the manner specified by the author or licensor

The original problem is that the Make author (Bre Pettis) failed to specify how to attribute and did not bother even including his name (I assume "Bre" is a boy's name).

Michael LeBlanc perpetuated the problem by not specifying how to attribute and also did not bother including his name in the source file.

The lack of attribution is understandable.  The original authors do not appear to have any interest in attribution.

Quote
Nearly everybody who shows this code, or a close facsimile of, seems to think that they actually wrote it...


That is painfully nauseating.  I suspect it's a good way to weed out low quality employees.

Nick Gammon

I always smile when I read this stuff:

Code: [Select]

  DDRB = 0x00;           // make PORTB all inputs
  DDRD = 0x00;           // make PORTD all inputs


It's like 0x00 is a "more powerful" form of zero. The concept is nicely sent up here:

http://www.youtube.com/watch?v=eaJtjJNrWf0

Quote

In addition to the normal zero, there is the double zero, that's for where things are really zero. This one's pretty zero (points to the single zero) and this one is absolute zero (points to the 00 key).
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

Nick Gammon

Also, the code that is badly formatted is probably a tip that it has been reblogged by someone without code tags, and thus the original formatting has been lost (and they didn't care).
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

Pedro147

Good detective work CB; have a gold star.I managed to find an earlier reference -
This from 2008
http://stationinthemetro.com/2008/01/20/makedc-led-cube-workshop
Linked to this
http://makezine.com/2007/09/06/make-a-pocket-led-cube-we/

Nick - we knew that you would never submit scrappy coding with poor indentation
http://www.pedroduino.com

JoeN


I always smile when I read this stuff:

Code: [Select]

 DDRB = 0x00;           // make PORTB all inputs
 DDRD = 0x00;           // make PORTD all inputs



It may also be that whoever is writing the code has chosen hex because it does make sense to use it when setting and resetting individual bits as long as you know what hex codes map to which nibble bit patterns.  And then you just end up using one numbering format for consistency sake.
I have only come here seeking knowledge. Things they would not teach me of in college.

Nick Gammon

No doubt someone will be able to Google an example of where I did exactly that myself. :)
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

cjdelphi

You'll find a lot of "good" code is given out, but their *best* code is not.

Go Up