Writing to SD after removal/reinsert

geo_leeman

I'm switching a data logger over to C++ (yay!) from spin, but am hitting an odd behavior in the SD card writing bit. I've got a logging switch (on/off) so the user could turn it off, remove the card, insert a new one (or the same really), and turn it back on. I can turn it off/on to my heart's content, but as soon as I remove the card, it's a no go with no further writes occurring. I've simplified it to the program below to demonstrate the issue. I let this run a few loops, then remove the card during the period after the file is closed (and my safe to remove LED is on). Upon reinsertion, no more writes actually happen. Any ideas? Thanks!

#include <stdio.h>            
#include <propeller.h>       

#include "simpletools.h"                      

int main(void)                               
{
  while(1) {
    sd_mount(19, 20, 21, 22);                 
    FILE* fp = fopen("test.txt", "a");          
    fwrite("Testing 123...\n", 1, 15, fp);      
    fclose(fp);
    high(7);  // LED for safe to remove ON                                
    print("2 second pause\n");     
    pause(2000);
    low(7);  // LED for safe to remove OFF
  }   
} 

DavidZemon

I'm fairly sure that the SD/FAT library in Simple + PropGCC uses a separate cog for the SPI and low-level SD interface. I don't see any code to shut that cog down before starting up the next one. sd_mount invokes dfs_mount and does what looks to be a bit of house keeping, but I think the core logic is the dfs_mount() invocation. Unfortunately, that function is defined in PropGCC itself and I don't feel like digging through its source code right at this moment, so I have no idea if that function has the smarts necessary to ensure only one driver cog is going at a time. If it doesn't have the required smarts, then my guess would be that the CS line is being held high by the first cog (because nothing is communicating with it anymore) and therefore the SD card stops responding to all further requests.

Not sure it will help any, but do check the return code of sd_mount and see if its throwing any errors.

If this ends up being a feature that isn't supported by Simple + PropGCC, you have a few other alternatives: convert a Spin object to C++ with spin2cpp, use PropWare's SD/FATFS library, use libpropeller's SD/FATFS library

Dave Hein

The problem is that the simple libraries don't have an sd_unmount routine. In addition to calling dfs_mount, sd_mount also adds a link to the file driver struct to the driver list. An unmounts routine needs to remove the file driver from the list and call dfs_unmount. I believe this would allow you to remove the SD card, and then re-insert it and mount it again.

geo_leeman

DavidZemon wrote: »

Not sure it will help any, but do check the return code of sd_mount and see if its throwing any errors.

I don't believe it was returning an error code.

Dave Hein wrote: »

The problem is that the simple libraries don't have an sd_unmount routine. In addition to calling dfs_mount, sd_mount also adds a link to the file driver struct to the driver list. An unmounts routine needs to remove the file driver from the list and call dfs_unmount. I believe this would allow you to remove the SD card, and then re-insert it and mount it again.

Ah - I'll have a look at making some kind on unmount then and update with the results.

Rsadeika

Hopefully you will be sharing the code for an unmount function.. In my project, that was going to be the next question that I was going to pursue. Getting a little tired of having to re-run the program after I extract and then re-insert the card.

Ray

geo_leeman

You bet I'll share and make a PR so it can eventually just be part of the distro.

geo_leeman

Okay so, dfs_mount (https://github.com/parallaxinc/propgcc/blob/5fc1abdabd91af35f92540696466b523333480b4/lib/drivers/mount.c) goes down to LoadSDDrive (https://github.com/parallaxinc/propgcc/blob/5fc1abdabd91af35f92540696466b523333480b4/lib/drivers/load_sd_driver.c) which doesn't appear to be very smart or provide a way to get ahold of the cog to kill it externally. Is this something propware would handle @DavidZemon?

DavidZemon

geo_leeman wrote: »

Okay so, dfs_mount (https://github.com/parallaxinc/propgcc/blob/5fc1abdabd91af35f92540696466b523333480b4/lib/drivers/mount.c) goes down to LoadSDDrive (https://github.com/parallaxinc/propgcc/blob/5fc1abdabd91af35f92540696466b523333480b4/lib/drivers/load_sd_driver.c) which doesn't appear to be very smart or provide a way to get ahold of the cog to kill it externally.

Ouch. That's too bad

geo_leeman wrote: »

Is this something propware would handle @DavidZemon?

Yes. In fact, PropWare doesn't even need to run anything in a separate cog. If you just want an exact copy of your test program above but all running in a single cog, then try this: (warning: untested)

#include <PropWare/hmi/output/printer.h>
#include <PropWare/memory/sd.h>
#include <PropWare/filesystem/fat/fatfs.h>
#include <PropWare/filesystem/fat/fatfilewriter.h>

using PropWare::SD;
using PropWare::FatFS;
using PropWare::BlockStorage;
using PropWare::FatFileWriter;
using PropWare::Pin;

int main () {
    const Pin led (Pin::Mask::P7, Pin::Dir::OUT);
    led.set();

    const SD driver;
    FatFS    filesystem(driver);
    FatFileWriter writer(filesystem, "new2.txt");

    while (1) {
        led.clear();
        filesystem.mount();
        writer.open();
        writer.puts("Testing 123...\n");
        writer.close();
        filesystem.unmount();
        led.set();
        pwOut << "2 second pause\n";
        waitcnt(CNT + 2*SECOND);
    }
}

But, if you'd like async writes, you'd want something like this (warning: untested)

#include <PropWare/hmi/output/printer.h>
#include <PropWare/memory/sd.h>
#include <PropWare/filesystem/fat/fatfs.h>
#include <PropWare/filesystem/fat/fatfilewriter.h>
#include <PropWare/concurrent/runnable.h>
#include <PropWare/utility/collection/charqueue.h>

using PropWare::SD;
using PropWare::FatFS;
using PropWare::BlockStorage;
using PropWare::FatFileWriter;
using PropWare::Runnable;
using PropWare::CharQueue;
using PropWare::Printer;
using PropWare::Pin;

class AsyncFileAppender: public Runnable {
    public:
        AsyncFileAppender (const char filename[], CharQueue &queue, volatile bool *attemptToMount)
            : Runnable(this->m_stack),
              m_led(Pin::Mask::P7),
              m_filesystem(this->m_driver),
              m_writer(this->m_filesystem, filename),
              m_queue(&queue),
              m_attemptToMount(attemptToMount) {
        }

        virtual void run () {
            PropWare::ErrorCode err;

            this->m_led.set_dir_out();

            while (1) {
                // Continuously attempt to mount until it succeeds. This should enable crude "auto-mount" on card
                // insertion
                do {
                    err = this->m_filesystem.mount();
                } while (err);
                this->m_led.set();

                this->m_writer.open();

                // Continuously empty the queue, so long as an unmount has not been requested
                while (*this->m_attemptToMount)
                    while (this->m_queue->size())
                        this->m_writer.put_char(this->m_queue->dequeue());

                // Give it a chance to empty the queue one last time
                const auto  remainingCharacters = this->m_queue->size();
                for (size_t i                   = 0; i < remainingCharacters; ++i)
                    this->m_writer.put_char(this->m_queue->dequeue());

                this->m_writer.close();
                this->m_filesystem.unmount();
                *this->m_attemptToMount = false;
                this->m_led.clear();
            }
        }

    private:
        uint32_t m_stack[256]; // FIXME: Untested stack size. Might need to be bigger... might be way too big

        const Pin     m_led;
        const SD      m_driver;
        FatFS         m_filesystem;
        FatFileWriter m_writer;

        CharQueue     *m_queue;
        volatile bool *m_attemptToMount;
};

int main () {
    const char    filename[]     = "test.txt";
    volatile bool attemptToMount = true;
    char          queueBuffer[256];
    CharQueue     queue(queueBuffer);

    AsyncFileAppender fileAppender(filename, queue, &attemptToMount);
    Runnable::invoke(fileAppender);

    const Printer printer(queue, false);

    while (1) {
        printer << "Testing " << 123 << "...\n";
        attemptToMount = false;
        waitcnt(CNT + SECOND * 2);
    }
}

Code size for these two examples is as follows:
Single cog: Code size = 13,952, Total size = 15,328
Multi cog: Code size = Code size = 15,224, Total size = 16,600


The unfortunate thing about this is that PropWare isn't very Windows-friendly. If you're on Windows, I'd recommend libpropeller's SD class. I checked - it has an unmount method and the docs even make explicit mention of freeing up the driver cog.

Dave Hein

I wrote the mount code, and it looks like I neglected to include an unmounts routine. I think the easiest approach would be to modify LoadSDDriver so that it saves the cog number in a global variable, such as dfs_cognum. Then we need a dfs_unmount routine that would stop the cog and clear dfs_cognum and dfs_mountflag.

DavidZemon

Dave Hein wrote: »

I wrote the mount code, and it looks like I neglected to include an unmounts routine. I think the easiest approach would be to modify LoadSDDriver so that it saves the cog number in a global variable, such as dfs_cognum. Then we need a dfs_unmount routine that would stop the cog and clear dfs_cognum and dfs_mountflag.

Could be global, could be static to the function. Probably gets compiled to the same thing?

@geo_leeman, if you prefer to stick with the Simple + PropGCC approach, you probably want to just override the function. If you define your own version of dfs_mount with exactly the same signature, the linker will (should?) select your function instead of the one provided by whatever lib PropGCC has it in. Then of course you can define a dfs_unmount as appropriate and invoke it.

Dave Hein

I don't think you would need to modify dfs_mount. You would need to your own copy of load_sd_driver.c that saves the cog number. In addition to writing a dfs_unmount routine you would also need to write an sd_unmount function that calls dfs_unmount and removes the file driver from the list. Basically, you want to undo what sd_mount does.

geo_leeman

Okay, I have a working prototype here, but some linking weirdness is still occuring. The code below does allow removal/reattachment of the SDCard (yay!) but you can see the cogstop is hard-coded. My LoadSDDriver is never getting called, but the one from the core library is being used. Is there a way to force the linking here? You can see I've included other functions like sd_mount verbatim to get around this, but it seems like the wrong solution.

#include <stdio.h>            
#include <propeller.h>       

#include "simpletools.h"                      
 

/*
 * Library Overrides and Additions for Unmounting
 */



volatile static int sd_cog;
extern uint32_t *_sd_mbox_p;
static volatile uint32_t __attribute__((section(".hub"))) sd_mbox[2];

void LoadSDDriver(uint32_t configwords[2])  // Not being called!
{
    print("My LoadSdDriver\n");
    use_cog_driver(sd_driver);
    uint8_t *driver_array = (uint8_t *)get_cog_driver(sd_driver);
    
    memcpy(driver_array + 4, configwords, sizeof(uint32_t) * 2);

    _sd_mbox_p = (uint32_t *)sd_mbox;
    sd_mbox[0] = 1;
    sd_cog = cognew(driver_array, (uint32_t *)sd_mbox);
    while (sd_mbox[0]);
}

void remove_driver(int i) {
  print("My remove_driver\n");
  _driverlist[i] = NULL;
}


static int sd_driver_idx;
extern _Driver _FileDriver;

extern _Driver _SimpleSerialDriver;

const int DRIVER_LIST_SIZE = 16;

_Driver *_driverlist[] = {
    &_SimpleSerialDriver, 
    NULL, NULL, NULL, NULL,
    NULL, NULL, NULL, NULL,
    NULL, NULL, NULL, NULL,
    NULL, NULL, NULL
};

int add_driver(_Driver *driverAddr)
{
  print("My add_driver\n");
  int i;
  for(i = 0; i < DRIVER_LIST_SIZE; i++)                      
  {
    if(_driverlist[i] == 0) break;
  }
  //print("i = %d", i);
  //print("\n");
  _driverlist[i] = driverAddr;
  return i;
}


int sd_mount(int doPin, int clkPin, int diPin, int csPin)
{
  print("My sd_mount\n");
  _SD_Params* mountParams = (_SD_Params*)-1;
    
  static _SD_SingleSPI sdPins;
  sdPins.MISO = doPin;
  sdPins.CLK = clkPin;
  sdPins.MOSI = diPin;
  sdPins.CS = csPin;    
    
  static _SD_Params params;
  params.AttachmentType = _SDA_SingleSPI;
  params.pins.SingleSPI = sdPins;
    
  mountParams = &params;

  if (mountParams == (_SD_Params*)-1)
  {
      return -1;
  }

  uint32_t mountErr = dfs_mount(mountParams);
  if (mountErr)
  {
      //print("Mount error: %d\n", mountErr);
      return mountErr;
  }

//  print("done.\n\n");
  add_driver(&_FileDriver);
  
  return 0;
}


extern int dfs_mountflag;

void dfs_unmount(void) {
  print("My dfs_unmount\n");
  dfs_mountflag = 0;
  print("%d", sd_cog);
  cogstop(1);
}  


void sd_unmount(void) {
  print("My sd_unmount\n");
  dfs_unmount();  // Will shut down cog
  remove_driver(sd_driver_idx);
} 
  
  
/*
 * Main
 */

int main(void)                               
{
  while(1) {
    sd_mount(19, 20, 21, 22); // Leeman Geophysical Logger
    // sd_mount(12, 13, 11, 8); // ASC+ Board                
    FILE* fp = fopen("test.txt", "a");          
    fwrite("Testing 123...\n", 1, 15, fp);      
    fclose(fp);
    high(7);  // LED for safe to remove ON                                
    print("2 second pause\n");     
    pause(2000);
    low(7);  // LED for safe to remove OFF
    sd_unmount();
  }   
}