Implement SD card handling

[Traumflug]

If you pull regularly, you might have noticed a new sdcard2 branch. This is for implementing handling an SD card. I reviewed and wrote the code carefully, but I have no such hardware, so I rely on your testing.

Other than the 3.5 years old(!) sdcard branch it uses not FatFs, but Petit FatFs. This didn't exist back in 2011 and is a slimmed down version of FatFs. Slimmed down, like it can't handle file metadata and can handle one file at a time only, which is entirely sufficient for our needs.

Current status is, M21 - M25 implemented, so you should be able to open an existing file and print from it. To enable this stuff, define something like this in your config.h:

#define SD_CARD_SELECT_PIN    DIO15
#define SD_CARD_DETECT_PIN    DIO16

The latter only if you actually have a SD card detect pin. Pin names have to match your actual hardware, of course.

The only supported filesystem is currently FAT16. Expanding to also FAT12 and FAT32 shouldn't be difficult, there are flags in pff_conf.h. Implementing file writing will be a bit more difficult, but I guess I'll do that as soon as reading is known to work.

The nice thing is, all this stuff currently chimes in at only 3156 bytes, so Teacup still easily fits into ATmega 328's. Not defining a select pin entirely removes all the SD card support, not a single extra byte if you don't need such stuff.

[Traumflug]

A few smaller improvements done in-place, please pull this branch again.

[jbernardis]

I tried to exercise this code today, but as soon as I defined SD_CARD_SELECT_PIN (in my case as DIO53) I was unable to establish communications with the firmware from my host. I'm talking about just opening the port - not sending commands. As soon as I comment out this one line, everything works again. I'm not sure what I should try to get past this. Obviously I can't make use of any debugging if I don't have basic serial communicaitons.

Any suggestions?

[Traumflug]

Thanks for testing. I see no other chance than to comment out the new code (in sd.c and pff*.c) partially to narrow down what blocks the serial port. If the serial port keeps working with the code in sd_init() commented out, comment it back in and comment parts of pf_mount(), and so on.

[jbernardis]

I'm getting to the first call to xchg_spi in disk_initialize. This seems to never return. It is a short routine that just moves the output character into the send register, and then calls loop_until_bit_is_set waiting for input. I don't think this loop is ever terminating.

DSTATUS disk_initialize(void) { BYTE n, cmd, ty, ocr[4];

ifdef SD_CARD_DETECT_PIN

/\* Original code uses an 10 ms repeating timer to set/clear STA_NODISK
   and STA_PROTECT (write protection). We use no such timer for better
   performance, at the risk that these flags are not always up to date. _/
if ( ! READ(SD_CARD_DETECT_PIN)) { /_ No card in socket. */
  card_status & STA_NODISK;
  return card_status;
}

endif

power_on(); FCLK_SLOW(); for (n = 10; n; n--) /* 80 dummy clocks */ xchg_spi(0xFF); <== we fail in here

Everything I've read about SPI tells me that we're doing the correct thing here. On the Mega 2560, the SD cad should be connected to D50(MISO), D51(MOSI), and D52(SCK). Further, the "standard" pin to use for chip select is D53. This is exactly how I'm connected. I did not define the card detect pin, but I have a card with a verified GCODE file on it inserted in to the reader. I reflashed my marlin setup, and the SD card works fine. So even though it's not an SDRAMPS, the pin config is the same so it ought to work in teacup as well.

The only element of doubt I have is the pin assignment. I am assigning Slave Select to DIO53. I am physically connecting to D53 on the AUX3 connector on the RAMPS. I assume D53 == DIO53

EDIT: I did a bit more exploring through the code, and I noticed that there is a collision between the MAX6675 code, and support for SDRamps - both use D53 as their Slave Select pin; one of them will need to move. That, however, is NOT my issue because I to not have TEMP_MAX6675 defined so that code is not present in my image.

[Traumflug]

At least some progress. (Always think positive)

Here's the corresponding section from the Arduino library, Sd2Card.cpp:

static void spiSend(uint8_t b) {
  SPDR = b;
  while (!(SPSR & (1 << SPIF)));
}
[...]

uint8_t Sd2Card::init(uint8_t sckRateID, uint8_t chipSelectPin) {
  errorCode_ = inBlock_ = partialBlockRead_ = type_ = 0;
  chipSelectPin_ = chipSelectPin;
  // 16-bit init start time allows over a minute
  uint16_t t0 = (uint16_t)millis();
  uint32_t arg;

  // set pin modes
  pinMode(chipSelectPin_, OUTPUT);
  chipSelectHigh();
  pinMode(SPI_MISO_PIN, INPUT);
  pinMode(SPI_MOSI_PIN, OUTPUT);
  pinMode(SPI_SCK_PIN, OUTPUT);

  // must supply min of 74 clock cycles with CS high.
  for (uint8_t i = 0; i < 10; i++) spiSend(0XFF);

Looks very similar to my eyes. Pinmodes are set in mendel.c, line 186..188. Except that Arduino uses a different order and writes no value to MISO/MOSI/SCK on initialisation.

Other than that I'm out of ideas other than googling and looking into Atmel Application Note AVR317.

[jbernardis]

I think I'm going to try to isolate the code onto my Uno. It very well may be my SD card, but if I can take a closer look at it maybe I can figure it out.

[jgrjgr]

i get the same behaviour where serial communications don't work at all when the sdcard stuff is enabled,

[Traumflug]

At least to my understanding, SPI shouldn't block even with no card being present at all and select pin misconfigured. You simply read all zeros, then.

[jbernardis]

Ok - I stripped the code down to very basics and am trying to run it on my Uno, using the serial port to debug, but I haven't even gotten started. Basically, I removed all of the logic from mendel.c except for initialization. Right now even the main loop is empty except for a delay_ms and a serial_write_str (not even any SD logic yet), but I never see any of the output on the terminal program I run. I know I'm in the program because i'm toggling the pin 13 LED, and I can see that happening.

I have the terminal program set to 115200-8-N-1, and I know there is connectivity because if I toggle DTR in the terminal program, I can see the Arduino reset.

Am I wrong in assuming that the serial.c routines just write out over the USB port? It ts, after all, the way teacup itself communicates with a host - using those same routines. Is there something else I need to be doing?

[Traumflug]

Am I wrong in assuming that the serial.c routines just write out over the USB port?

These routines write to a buffer and this buffer is forwarded byte by byte to the serial port by an interrupt. Same for receiving in the opposite direction, with another interrupt. So you won't see any transmssion with interrupts disabled. Serial port to USB conversion happens outside the ATmega, on a separate chip.

There isn't much SPI code around. These three lines in mendel.c and the stuff in sd/pff_diskio.c. To run one-off code simply put it into main(), just before the loop. At times I had whole math investigations there.

[jbernardis]

Thanks for the tip. I got the serial port working. No discoveries re the SD card yet, but at least I have a platform from which to do some testing.

[jbernardis]

I know why the firmware is hanging. It's a very simple fix. These lines:

        PRR = MASK(PRTWI) | MASK(PRADC) | MASK(PRSPI);
        PRR0 = MASK(PRTWI) | MASK(PRADC) | MASK(PRSPI);

near the top of io_init in mendel.c actually disable the SPI interface to save power. I changed them to:

        PRR = MASK(PRTWI) | MASK(PRADC);
                #ifndef SD
                    PRR |= MASK(PRSPI);
                #endif

        PRR0 = MASK(PRTWI) | MASK(PRADC) | MASK(PRSPI);
                #ifndef SD
                    PRR0 |= MASK(PRSPI);
                #endif

and this allows the code to proceed. The problem is that it still fails to mount the card. I bored down to the lowest level - the code is actually pulling the SD_CARD_SELECT_PIN low and waiting for acknowledgement from the card - which never comes. I'm not sure what to do next. I think I need to check to see it the select pin is actually going low - it might require a bit of breadboarding. I just wanted to make this known in case someone out there with a real SD ramps card could give it a try.

Shall I push this change out to the 'sdcard2' branch?

[jbernardis]

I haven't been able to make any progress on this, so I have pushed up my change to mendel.c to not disable the SPI interface if SD card support is set.

[Traumflug]

Nevertheless, you're a genius. It likely had me taken weeks to find this.

You're on a RAMPS, aren't you? Can't you just measure this pin? All signal pins are forwarded to the Arduino, so they have to go through these big connector rows connecting these two boards.

[jbernardis]

I put an LED on the select line and indeed I saw it go low whenever I tried to mount the card, but I could get no further than that. I got to the select routine inside of pff_diskio, and it just timed out. I didn't know where to go next so figured I'd get everybody else's mind working on it too by pushing my change.

[jbernardis]

I know we haven't gotten to execute the code yet, but I think I see a potential issue with processing SD card information. It's in the main loop in mendel.c. The data being receivied over the serial port is peing processed one character at a time. Logically, that's also how data from the SD card is read. The problem is that if I type in a command interactively, or even if my host software sends out a temperature query (for example), the command from the serial port will be interwoven with the current command being read from the SD card. There is a comment to that effect about CANNED_CYCLE, but it could also bite us here.

The bottom line is that we have two streams of input data that are being handled simultaneously. I think we need some way of not switching from one to the other unless we are at the end of a line.

[Traumflug]

That's true. A lock flag for each of them cleared after reading a \n and raised after anything else might help.

[jbernardis]

I've worked out the following pseudocode. If you like I can translate to C and push, but I have no way of testing.

currentSource = NONE;

c = null;

while (1) {
    if (c != null)
        gcode_process(c);

    if (c == '\n' || currentSource == None) {
        if (c = readserial())
            currentSource = SERIAL;
        else if (c = readsd())
            currentSource = SDCARD;
        else
            currentSource = NONE;
    }
    else  {
        if (!queuefull()) {
            switch (currentSource) {
            case SERIAL:
                if (serial characters ready)
                    c = next char from serial port;
                else
                    c = Null;
                break;
            case SDCARD:
                c = next char from SD card;
                if (EOF) {
                    c = Null;
                    currentSource = NONE;
                }
            }
        }
    }
}

[Traumflug]

How about

uint8_t lock_serial = 0, lock_sd = 0;

for (;;) {
  if (queue_full() == 0) {
    unit8_t c;

    if ( ! lock_serial && serial_rxchars() != 0) {
      c = serial_popchar();
      gcode_parse_char(c);
      lock_sd = (c == '\n' ? 0 : 1);
    }
    if ( ! lock_sd && c = read_a_byte_from_sd()) {  // didn't look up the actual function
      gcode_parse_char(c);
      lock_serial = (c == '\n' ? 0 : 1);
    }

    // CANNED as before
  }
}

A remaining problem is, that when reading from SD, the queue will be always filled and characters from serial can slip in only when the bot finishes a movement to start the next. Impatient hosts might send more commands during a move than what fits into the serial buffer. One solution to this might be to accept characters on serial while printing from SD even when the queue is full, but to discard any commands which need queueing ( = movements).

[Traumflug]

BTW., yet another pitfall is, handling a command from SD card shouldn't send an "ok" back over the serial line.

I didn't look up how other firmwares handle this. Just yesterday a guy in the german forum lamented his controller running Marlin or Repetier would be "dead" when waiting for temperature or SD card uploads.

[jbernardis]

I like your logic, and I think the remaining problem you identify is mitigated as much as possible. At the end of every line from the serial link, the SD gets a chance to slip a command in and vice versa.

As far as the unneeded OK's, I believe that is sent in gcode_parse, so we'd need to pass in another parameter to gcode_parse telling it whether or not it should be silent. This code we're talking about above, I believe, is the only place calling gcode_parse, so it should be simple to add the parameter.

[triffid]

Perhaps the gcode parser state should be attached to each stream? What happens if we read a G91 from the SD card then a G1 from serial? Could have unexpected effects ;)

If parser state is attached to the stream, then it eliminates the need to avoid interspersing lines from multiple sources.

Also, you might like to consider passing a stream reference or write callback or something to the parser, for it to send not only "ok" but temperature responses and suchforth. The SD reader would simply discard the replies, or optionally write them to a file.

Since SD read can be so fast and you get 512-byte blocks, the serial won't have much opportunity to insert lines.

Perhaps mainloop should only read data from the SD when the queue is less than half full? That way, serial lines have a decent chance of not waiting to be queued, and groups of them have a chance to be executed together.

I haven't checked teacup queueing architecture recently, but if we don't queue temperature waits and G4 dwell and instead drain the queue then stall the relevant gcode source, we could have the added feature of being able to jog over serial while the SD print is temperature-waiting.

Such architectural changes would also be useful if someone wants to add more gcode sources, such as bluetooth on a 2nd serial port

[jgrjgr]

I want to propose a bit of a different route for sdcard and to an extent lcd handling in teacup,

given the dirt cheap cost of arduino unos and the vast availability of cheap HD44780 lcds which are in a shield configuration complete with buttons shouldn't we be looking at using a second uno to handle the lcd and sdcard ? then the communication between them can run on the existing serial interface or better yet a spi interface with crc and error checking ability and we can ultimately maintain teacups nice smooth movement and acceleration..... call me mad but it could also be usefull on other serial compatible boards

[Traumflug]

@triffid

What happens if we read a G91 from the SD card then a G1 from serial? Could have unexpected effects ;)

Indeed. But you have to allow /some/ commands over the serial line, e.g. "stop printing from SD card".

Since SD read can be so fast and you get 512-byte blocks, the serial won't have much opportunity to insert lines.

As serial comes first in the loop, serial would be drained each time a \n is read from SD.

I haven't checked teacup queueing architecture recently, but if we don't queue temperature waits and G4 dwell and instead drain the queue then stall the relevant gcode source, we could have the added feature of being able to jog over serial while the SD print is temperature-waiting.

Now it becomes tricky. If we allow to jog/move, what holds the user back from starting yet another print over the serial line? At first glance I think it's a matter of which command serial accepts while having an SD file open and which not.

Queue architecture is the same as 3 years ago, just passing homing flags around was added at some point.

And this stream reference is certainly a good idea, if just in the primitive form of a flag "serial yes/no" for now. Likely it performs better if this flag isn't passed around through all the function calls, but kept in a global place. Not sure wether that's possible, though.

@jgrjgr

given the dirt cheap cost of arduino unos ... shouldn't we be looking at using a second uno to handle the lcd and sdcard ?

This should work already. I'd prefer something like a RaspberryPi, though, much more bang (Ethernet, touch display capability, etc.) for only few more bucks. If you really want an Arduino, this thing would need an entirely different firmware. One which handles the display and forwards SD card files to the serial line.

[phord]

I think the "command stream source" issue needs to be sorted out better than this. With the current suggested code, if I read it right, a single character received from the serial port which is not '\n' will pause the sdcard print since it will result in lock_sd=1. On line-oriented comms programs, this might not ever happen, but character-oriented comms could stall the print easily, as could spurious line noise. I suppose it is a trade-off the user must accept if he wants to jog during dwells, for instance.

I think triffid's stream reference idea might save the day. Since the parser is separated so well from from the executor, it seems it would be possible to parse serial commands in one context and sdcard commands in another. I experimented with this some and it was pretty straightforward to start. All the commands from different streams just affect their own GCODE_COMMAND structure.

But the 'next_target' global gets in the way because each new command should begin with the state left from the previous one. When commands can be executed asynchronously, the "previous state" might change between the time a command begins and when it is complete. :-(

Anyway, I was only playing around and I don't have access to my printer right now. I pushed some POC code to the 'issue-104' branch to demonstrate my thinking.

[Traumflug]

Thanks, @phord, excellent demonstration. Link: https://github.com/Traumflug/Teacup_Firmware/commit/b31110d88cba3b7e4e7531fbfde0445845f5ecbb

[Traumflug]

Just rebased branches sdcard2 and issue-104 to experimental, so both come with Configtool now.

[Traumflug]

Uhm, anybody having fun in placing a selector for SD_CARD_SELECT_PIN in Configtool? Perhaps also a choice to select wether read-only or read-write.

[Traumflug]

As I'm apparently the only specimen of mankind capable to implement SD Card support into Teacup I took this to my chest again, while also soldering kind of a steampunk SD Card adapter (photo forthcoming). The result is on the sdcard3 branch.

Testing goes so far only up to SD card: add the layer between hardware and Petit FatFs. It should list all the files in the top level directory every 5 seconds. You're welcome to test. If it doesn't work, you can go back a few commits and see wether this works. Each commit comes with some demonstration code, so you can easily see wether it works or not.

To enable this feature, a #define SD_CARD_SELECT_PIN has to be in your board configuration. This currently also disables printing, so don't be surprised if the size of the binary shrinks considerably.

[jgrjgr]

do you want me to send you a proper sdcard reader?

[youssefaly97]

I volunteer to test the SD card implementation

[Traumflug]

do you want me to send you a proper sdcard reader?

Thanks for the offering, this steampunk adapter works well. A simple check on your controller would be more helpful.

I volunteer to test the SD card implementation

Excellent! What is the result? Code is on the sdcard3 branch, don't forget the SD_CARD_SELECT_PIN.

[youssefaly97]

SD Card Mounted Successfully message appears, displays /SYSTEM~1 and /PIPE-T~1.gco. first file is a hidden system file, second one is a gcode file.

I tried only FAT16 file system, I understand from your previous post that FAT32 needs minor code changes that I'm not sure of.

FAT16 limits maximum partition size to 2GB, an SD card can have more than one partition but windows can only read the first, so I forcefully formatted my 8GB card to 2GB. Also FAT16 limits file names to 8.3, which is 8 characters for the name and 3 for the extension while gcode files end with .gcode which is 5 characters. I think we strongly need the FAT32 support.

[youssefaly97]

All motors energized, all mosfets on. I don't know about the rest as this is the only hardware connected at the moment.

[Traumflug]

Thank you, @youssefaly97. Looks like it makes sense to continue from here.

Regarding FAT32 support: it's a simple #define-switch in pff_conf.h. Should work, but won't give you long filenames. Long filenames are pretty resource intensive, IIRC. If you're aware of an implementation on AVR (e.g. a firmware supporting it), please let me know.

[Traumflug]

All motors energized, all mosfets on.

With SD Card switched on, printing is currently switched off. Not much of a problem, just work to be done.

[youssefaly97]

Anytime, @Traumflug . Yes, seems reasonable.

I think we should only use FAT32 for the extension, not to mess up when loading files on it and leaving space larger than 2GB per partition. Long file names need UTF-16 characters so it'll eat up a lot of RAM and Flash which contradicts with the whole firmware targets.

[youssefaly97]

The printing switched off thing, I think causes the outputs to be in high-Z mode so the enable pins on the step sticks are floating and causing the motors to be partially energized.

[Traumflug]

Hmm. How should M20 list files? RepRap wiki says "ok Files: {SQUARE.G,SQCOM.G,}", but looking at Sprinter's implementation I can see neither a "Files:" nor a comma.

Does the format matter ( = do hosts read the list) ? If not, I'd prefer a more human readable list (one name per line).

[jgrjgr]

i'm not sure the reprap wiki is very up to date on that particular bit, smoothie and marlin from what i've seen in pronterface simply dumps a "/n" seperated list of the files then the host program is expected to decipher it

[youssefaly97]

I think the list should stored in a string so it's compatible with both serial and lcd interfaces, something universal.

[jgrjgr]

how muchmemmory is available for that sort of thing ?

[jgrjgr]

i just did a quick test with a terminal program on marlin with M20, this is what it outputs

ok M20 Begin file list /PRUSAI2/PI2PLA~1.GCO /PRUSAI2/PRUSAI~1.GCO /PRUSAI2/PLATE4~1.GCO /PRUSAI2/Z-MOTO~1.GCO /PRUSAI2/YSENSO~1.GCO /POSSUM/BARCLA~1.GCO /POSSUM/IDLERP~1.GCO /POSSUM/RIGHTT~1.GCO /POSSUM/ZCARRI~1.GCO /POSSUM/Z-MOTO~1.GCO /POSSUM/ZCARRI~2.GCO /POSSUM/CARRIA~1.GCO /POSSUM/CARRIA~2.GCO /POSSUM/RUMBAB~1.GCO /POSSUM/COVER3~1.GCO /POSSUM/PTFECO~1.GCO ZCARRI~1.GCO ZCARRI~2.GCO ZADJUS~1.GCO CARRIA~1.GCO CUBE~1.GCO FEET~1.GCO GEAR_S~1.GCO End file list ok

[youssefaly97]

Well, we can store the string in the program memory (flash) but that would be slow and would take up the number of read and write cycles fast, eeprom same, ram if the string is large enough, it will freeze. I think we have a sort of buffering string, one file name is read from the sd card into that string, displayed through the available/selected interface, string cleared for next file, advance to next file and repeat.

[Traumflug]

i just did a quick test with a terminal program on marlin with M20, this is what it outputs

Thanks. It works this way now, except for the start/end markers. Whenever they become neccessary it's trivial to add them.

About storing: an SD card IS flash, so these names are actually stored in flash already. No problem to retrieve them over and over again.

Today I implemented M20 (file list, needs M21 before), M21 (card init), M22 (card close (which is almost useless)) and M23 (file open). The latter tries to open the file and shows its size to prove the code works. Not to forget, you can print from serial now even with SD_CARD_SELECT_PIN defined. Firmware size is about 21k with SD support.

All on the sdcard3 branch. In case you want to try, please also try to abuse it. Like using no or a too long file name or whatever nonsense could happen. Printer shall not go up in flames in such cases :-)

[youssefaly97]

That's great.

What I meant by flash is the atmega's flash.

I'm sorry but I'm currently out of town, will be back on Wednesday and I'll test it then.

[Traumflug]

Just "printed" the first file from SD card on the test bench. All on yet another branch, sdcard4.

M20 to M25 implemented. Depending on the card it still might require enabling FAT32. Will turn back to this later.

• Processing happens on a per-line basis. While printing from SD, typing something on the serial line will stop reading from SD until this line is done. Simple way to not mix up G-code coming from both sources.
• M23, then M24 also restarts a file.
• M26 will likely be added, easy to implement.
• Not sure what M27 should read. RepRap wiki says "report status". Whatever "status" means.
• M20 currently lists the top level directory, only. Also, printing only works from there. Can't afford to reserve 1024 bytes RAM just to receive a full path :-)

Enjoy!

[jgrjgr]

this is fantastic news :D , does the fat32 implementation cost much more if any program memmory by comparison to fat16?

[Traumflug]

Measured exactly this just a minute ago:

/*---------------------------------------------------------------------------/
/ File system support.
/
/ Firmware binary sizes with read-only on AVR ATmega are are as following:
/
/              only   plus FAT12   plus FAT16   plus FAT32   plus both
/   FAT12   (21810)        - - -         + 98        + 546       + 660
/   FAT16   (21718)        + 190        - - -        + 504       + 752
/   FAT32   (21996)        + 360        + 226        - - -       + 474
/---------------------------------------------------------------------------*/

Glad to see FAT32 doesn't add too much, so I enabled it (by default). Didn't add FAT12, these 248 bytes seem to be pretty much a waste. FAT12 supports only volumes up to 32 MB (or 12 or 16 MB, depending on source).

Good choice?

[jgrjgr]

i think it is a good choice,