As part of my LED strip project I needed a 7-segment display capable of displaying two or more digits. I could have built something myself, but I'm short of time as it is so the SparkFun kelly green seven-segment serial display seemed like a good trade-off between cost and speed of implementation. It can be driven either over a serial link or via SPI. SparkFun also do the same unit in yellow, red and blue. The units use a ATMega328 to drive the display, which is the same microcontroller that's in the Arduino Pro Mini's that I'm using for the project. I therefore bought two of them from one of SparkFun's distributors in the UK.
As I'm already using SPI for most of the peripherals, the obvious thing to do was to use the display's SPI interface to drive it. I spent an absolute age trying to get the display to work, but no matter what I tried, the output on the display was a seemingly random collection of characters and garbage. After a lot of wasted time I eventually got it to work most of the time by dropping the SPI clock rate down from the specified 2MHz to 0.5MHz, but even at that it was still glitchy.
I tried unsuccessfully over a period of several weeks to get some help from SparkFun with the problems I was having, first trying the SparkFun IRC channel, then an unanswered question on the product page and then and equally unanswered forum post. In the end I emailed their tech support group, and finally got a response. A 30-mail thread then ensued, with the tech support person playing piggy-in-the-middle between mself and the SparkFun engineer who is responsible for the product. They had no clue as to what the problem was - initially I suspected that the CLKDIV8 fuse bit had been mis-programmed as the SPI interface would only work at approximately 1/8th of the specified clock rate, but as I didn't have anything capable of reading them I couldn't be sure. Plus, if that was the case I'd expect the serial interface not to work properly either, which wasn't the case. The engineer then said he thought that the problem was that the boot loader fuse bits were wrong. I thought that was hokum as the device doesn't use a boot loader, but they wanted to send me two new units with the 'fixed' fuse bits which they assured me would fix the problem. I asked instead if they could send me a STK500 programmer instead so I could fix the fuse bits myself and end up with something I was probably going to buy anyway but they refused because it was "Too difficult" to arrange. I wasn't happy, but agreed to try two new displays.
The displays duly arrived, and to my complete lack of surprise they were just as broken as the original ones. I fed this back to SparkFun tech support who duly admitted they were completely stumped. Great.
I then did something I should have done in the beginning, and downloaded the firmware source for the displays and had a look, at which point the cause of the problems became immediately apparent. Oh boy, what a complete pile - probably some of the shoddiest code I've seen in quite some time. Here's a brief description of the issues I found:
- The immediate cause of the problem is the ISR that services the SPI interface. The ATMega328 is being clocked at 8MHz using the internal oscillator, and when in slave mode the maximum SPI data rate is FCPU/4, i.e. 2Mhz. Each SPI transaction transfers one byte, which works out at 32 CPU clock cycles per byte. That means the ISR has to use less than 32 clock cycles if it isn't to drop data. The ISR in the code is much longer than that, not least because it calls a two-page subroutine for each processed byte!
- The firmware consists of one .c and one .h file, with all of the global variables and about a quarter of the functions being defined in the .h file rather than the .c file. Huh?
- Globals are shared between the ISRs and the main loop without any of them being declared volatile. Plus there's no locking whatsoever around any of the shared multibyte globals - see the avr-libc documentation for an explanation of why this is a problem.
- Many of the globals are unnecessarily long, e.g. 16 bits to hold a value that ranges from 0 to 4. Also, multiple variables are used where a single state variable would be sufficient.
The timer delay code is completely bizarre. There's a
delay_us()microsecond delay function that takes a delay of up to 2^16 microseconds, yet the
delay_ms()millisecond delay function consists of a string of four
delay_us(250)calls in a loop rather than a single delay_us(1000) call. No idea why. Oh, and the comments (such as they are) make absolutely no sense.
There are many other doozies as well, for example
switchstatements that only have 1
case, blocks of code where some members of a set of conditionals are handled by a
switchblock and others are handled by
I proved that the ISR overrun was the cause of the problem rather than the SPI data rate by leaving the SPI data rate at 2MHz in the master and inserting a delay between each SPI transaction. With a 15msec delay, the driver ISR can keep up and the display works correctly.
I fed back the information about the ISR overrun to SparkFun, and included pseudocode for a suggested fix. The reply I got back was that the engineer had "checked the clock speed again and claimed that the SPI bus was talking at 2MHz". Well duh - yeah. I also asked when I could expect a fix, the reply being that the tech support person had asked the engineer and that "he said it is on his list to review and rewrite the ISR. So no ETA, could take months.". I pushed back again, saying that at least the product pages and datasheet should be updated. As of writing, the product pages still don't note this issue, but the datasheet has been updated to say that the SPI interface won't run above 250kHz, so that's one small step, at least.
I really don't think this sorry saga is acceptable. Yes, SparkFun has offered me a refund, but I'm not quite sure how that's going to work out when I bought the parts via a distributor - when I asked for a STK500 as compensation, they cited the fact that I'd gone through a distributor as a reason why I couldn't return the original displays to SparkFun. I've subsequently bought a STK500 anyway so when I eventually find the time I can fix the firmware myself and reprogram the displays, but I really don't think that having to go down that route is acceptable. Plus I've wasted many hours trying to get these bloody things to work, and in the end I diagnosed the faults in their product - their engineer had absolutely no clue as to why it didn't work.
As I was still unhappy with the response I'd had, I mailed their customer support manager just under two weeks ago, and as of the date of this post, I've still not had a reply, which is just adding insult to injury.
I've always been a SparkFun fan in the past, but no more. Their hardware seems fairly well put together, but if the firmware I've looked at is representative, anything with a software component is going to be of extremely dubious quality, Plus their tech support truly sucks - they had no clue about what the problem was, and as I said I've had no reply from the support manager,