10 replies to this topic

[Kenn Sebesta]

This is a split of the thread http://forums.openpi...urrent-monitor/ Specifically, we're trying to create a generic I2C sensor reading module, one which will be able to beam back arbitrary data to GCS. The goal is not to interpret the data on module (that will depend on the particular user and so there's no way to do this in firmware, although with OP Revolution there is scripting support for PyMite).

I think it's very important that the user not have to recompile or download branches in order to use these modules. The individual sensor settings should be configured in a text file that is uploaded via GCS. It should be something that can be explained in a simply, for which we could indeed offer several configuration files for commonly used I2C sensors. Otherwise, the average user will simply feel overwhelmed by the process.

This is my idea for working with generic sensor reading:

  //Generic I2C addressing
  if( i2cStep == WAIT_STATE_0 ){
	// Block for WAIT_DELAY_0 [ms].
	xDelay = WAIT_DELAY_0 / portTICK_RATE_MS;
	vTaskDelay(xDelay);
  }
  else if( i2cStep == WAIT_STATE_1 ){
   // Block for WAIT_DELAY_1 [ms].
   xDelay = WAIT_DELAY_1 / portTICK_RATE_MS;
   vTaskDelay(xDelay);
  }
  else if( i2cStep == WAIT_STATE_2 ){
   // Block for WAIT_DELAY_2 [ms].
   xDelay = WAIT_DELAY_2 / portTICK_RATE_MS;
   vTaskDelay(xDelay);
  }
  else if ( i2cStep == WAIT_STATE_3 ){
   // Block for WAIT_DELAY_3 [ms].
   xDelay = WAIT_DELAY_3 / portTICK_RATE_MS;
   vTaskDelay(xDelay);
  }
  else if (pass==FIRST_PASS){
   if ( i2cStep == CONFIG_REG_0 ){
	GenericI2CSetConfig(0, 0, 0, 0, 0);
   }
   else if ( i2cStep == CONFIG_REG_1 ){
	GenericI2CSetConfig(0, 0, 0, 0, 0);
   }
   else if ( i2cStep == CONFIG_REG_2 ){
	GenericI2CSetConfig(0, 0, 0, 0, 0);
   }
   else if ( i2cStep == CONFIG_REG_3 ){
	GenericI2CSetConfig(0, 0, 0, 0, 0);
   }
  }
  else{
   if ( i2cStep == WRITE_REG_0 ){
	WriteGenericRegisters(0x00, NUM_REGISTERS_WRITE_REG_0, bufferTemp);
   }
   else if ( i2cStep == WRITE_REG_1 ){
	WriteGenericRegisters(0x00, NUM_REGISTERS_WRITE_REG_1, bufferTemp);
   }
   else if ( i2cStep == READ_REG_0 ){
	ReadGenericRegisters(0x00, NUM_REGISTERS_READ_REG_0, bufferTemp);
	decipherGenericI2Cresponse(bufferTemp, 0, 0, 0, 0);
   }
   else if ( i2cStep == READ_REG_1 ){
	ReadGenericRegisters(0x00, NUM_REGISTERS_READ_REG_1, bufferTemp);
	decipherGenericI2Cresponse(bufferTemp, 0, 0, 0, 0);
   }
  }

(Don't mind all the 0's, they're just placeholders so that it compiles correctly and doesn't complain.)

This is just a rough sketch that shows where I'd like to go with this. My idea is that there are likely only a fixed number of different wait times likely to occur with any given I2C chip. For instance, the HMC5883 requires an initial 8.3 ms wait, and then after that 67.7 ms wait cycles between reads.

After that, there will be a vector of i2cStep values which states which order things will be followed. So it might say to WRITE_REG,  then WAIT_STATE_3, then READ_REG, then WAIT_STATE_4, then loop back to the beginning of the vector to start over again. This gives a lot of flexibility, although at the expense of RAM (which is in short supply on the CC).

So, in the interests of having a concrete example, let's assume that we're using the HMC5883. The Operational Examples on pg. 18 show that the chip can be configured by

• Waiting 0.0083 [s]
  
• Writing 0x02 0x00
  
• Waiting 0.067 [s]
  
• Reading
  
• Clocking out 6 bytes

So this gives

WAIT_DELAY_0=9; //in ms, rounded up
WAIT_DELAY_1=68; //in ms, rounded up
CONFIG_REG_0=0x02;
CONFIG_REG_1=0x00;
NUM_REGISTERS_READ_REG_0=6;

And then you create the initial "flat" vector...:

firstPassVector={WAIT_STATE_0, CONFIG_REG_0, CONFIG_REG_1};

and then the "loop" vector:

loopVector={WAIT_STATE_1, READ_REG_0};

Thoughts? Especially for how to reduce RAM consumption to a minimum?


P.S. Yeah, there's a bug in the fact that I'm calling CONFIG_REG_0 and giving it a value, too. It's too late for me to fix it, but tomorrow I'll certainly add some sort of VALUE_CONFIG_REG_0 to fix it)

P.S.S. There's a second bug in the way waits are implemented, it should likely be vTaskDelayUntil()

[naiiawah]

Kenn Sebesta, on 23 November 2011 - 04:07 AM, said:

Thoughts? Especially for how to reduce RAM consumption to a minimum?

One thing you could do is cut down the repeated code in the if/else if blocks.  Something like this for each section:

	int waitDelay[] = { WAIT_DELAY_const1, WAIT_DELAY_const2, WAIT_DELAY_const3, WAIT_DELAY_const4 };
	// Block for WAIT_DELAY_x [ms].
	xDelay = waitDelay[i2cStep] / portTICK_RATE_MS;
	vTaskDelay(xDelay);

The cost on the array is 4 words, versus probably around 10'ish ARM instructions for each of the if/else if blocks.  I'd have to see the assembly to get an exact byte count, but even with THUMB, I think you will have a savings with the array technique.

[Kenn Sebesta]

naiiawah, on 23 November 2011 - 05:42 AM, said:

One thing you could do is cut down the repeated code in the if/else if blocks.  Something like this for each section:

	int waitDelay[] = { WAIT_DELAY_const1, WAIT_DELAY_const2, WAIT_DELAY_const3, WAIT_DELAY_const4 };
	// Block for WAIT_DELAY_x [ms].
	xDelay = waitDelay[i2cStep] / portTICK_RATE_MS;
	vTaskDelay(xDelay);

The cost on the array is 4 words, versus probably around 10'ish ARM instructions for each of the if/else if blocks.  I'd have to see the assembly to get an exact byte count, but even with THUMB, I think you will have a savings with the array technique.

Interesting, I didn't know that. However, while I see how this will save flash space, I'm not quite sure I see how it will save RAM. Can you explain some more?

[psupine]

I think you'll want to make the array "const" too, as in

const int waitDelay[] = { WAIT_DELAY_const1, WAIT_DELAY_const2, WAIT_DELAY_const3, WAIT_DELAY_const4 };
		// Block for WAIT_DELAY_x [ms].
		xDelay = waitDelay[i2cStep] / portTICK_RATE_MS;
		vTaskDelay(xDelay)

Otherwise the array will be RAM based, but it would have been initialised with constants. It's down to compiler implementation too and I'm not certain what this compiler does. It might choose to put all data in RAM anyway, const or not.

Of course, someone might chime in immediately and tell me I'm wrong about this, in which case I'll learn something I thought I knew.

[Kenn Sebesta]

psupine, on 23 November 2011 - 07:21 AM, said:

I think you'll want to make the array "const" too, as in

const int waitDelay[] = { WAIT_DELAY_const1, WAIT_DELAY_const2, WAIT_DELAY_const3, WAIT_DELAY_const4 };
		// Block for WAIT_DELAY_x [ms].
		xDelay = waitDelay[i2cStep] / portTICK_RATE_MS;
		vTaskDelay(xDelay)

Otherwise the array will be RAM based, but it would have been initialised with constants. It's down to compiler implementation too and I'm not certain what this compiler does. It might choose to put all data in RAM anyway, const or not.

Of course, someone might chime in immediately and tell me I'm wrong about this, in which case I'll learn something I thought I knew.

I think that I didn't make clear enough an important feature of this system, which is the idea that the generic I2C function will use values read in from GCS. So there has to be a vector of things telling the I2C code where to go next. The way I see it, it's almost like programming assembler. This approach works for several different chips I've looked at, but I'm worried the vector will take lots of memory, especially for things like CONFIG which can have the memory freed after use. Right now, the vector will probably take 20-30 bytes, which is a lot all things considered.

Note, I'm not really familiar with the const command and its implications, so it might well be that it works in the framework of what I'm trying. I guess when I see "const" I think "constant", which I do not think describes how this vector of instruction variables will be.

[psupine]

You are right that I didn't understand your aim from the code snippet which looks like it is just loading a delay value as a function of i2cStep. When you are talking about "where to go next" I think you are describing a jump table, which is a very efficient dispatch mechanism, but I don't know enough about PiOS to know whether it's the best solution. I'm sure you know more about that than me.

The const keyword does mean "constant" and it tells the memory layout process that the object is not expected to change (in fact not allowed to change) and so can live in ROM. If you are planning on modifying the jump table or parameter table from GCS on the fly, it will have to live in RAM and "const" should be left off.

[Kenn Sebesta]

I uploaded the skeleton to kenz/genericI2CSensor

The file of interest is flight/Modules/GenericI2C_Sensor/generici2c.c

I'd be interested if anyone wants to give it a try, but I caution that it's in very raw form, so it might not be usable. I'm more interested in thoughts on this approach.

[naiiawah]

Kenn Sebesta, on 23 November 2011 - 05:59 AM, said:

Interesting, I didn't know that. However, while I see how this will save flash space, I'm not quite sure I see how it will save RAM. Can you explain some more?

Argh!  I keep forgetting that a large part (all?) of the flight code is running out of flash with CC.   In my normal (in real life) embedded programming world, code gets copied out of very slow compressed flash into RAM and run there, so you try to save as much on code space as you do on bss, stack, and heap.  You are right, it may not save much in RAM in CC, but it is worth looking at the generated assembly to see for sure.

[naiiawah]

Kenn Sebesta, on 23 November 2011 - 04:07 AM, said:

This is a split of the thread http://forums.openpi...urrent-monitor/ Specifically, we're trying to create a generic I2C sensor reading module, one which will be able to beam back arbitrary data to GCS. The goal is not to interpret the data on module (that will depend on the particular user and so there's no way to do this in firmware, although with OP Revolution there is scripting support for PyMite).

[...]

This is just a rough sketch that shows where I'd like to go with this. My idea is that there are likely only a fixed number of different wait times likely to occur with any given I2C chip. For instance, the HMC5883 requires an initial 8.3 ms wait, and then after that 67.7 ms wait cycles between reads.

After that, there will be a vector of i2cStep values which states which order things will be followed. So it might say to WRITE_REG,  then WAIT_STATE_3, then READ_REG, then WAIT_STATE_4, then loop back to the beginning of the vector to start over again. This gives a lot of flexibility, although at the expense of RAM (which is in short supply on the CC).

Hmmm, I'm still turning this one over in my head, so this is just "typing out loud"....  The baud rate (bits / sec) for Bluetooth is usually either 57600, or 115200.  If the tightest timing you have to manage is write/read a register after waiting around 8milliseconds then this might work.  In that 8 milliseconds, the telemetry link could have shipped almost 500 bits at 57600, double that at 115200.  It wouldn't take but a couple dozen to say wait N millisec, write val X or wait N' millisec, read X', return data.

Could you just have a simple state machine running on the CC side that responds to the individual steps in the protocol as you go?!?  All the intelligence is on the GCS side of the link, and it drives the protocol by "remote control" by shipping over these command packets?   Crazy idea?

Like I said, just thinking out loud.

[Kenn Sebesta]

naiiawah, on 24 November 2011 - 04:49 AM, said:

Hmmm, I'm still turning this one over in my head, so this is just "typing out loud"....  The baud rate (bits / sec) for Bluetooth is usually either 57600, or 115200.  If the tightest timing you have to manage is write/read a register after waiting around 8milliseconds then this might work.  In that 8 milliseconds, the telemetry link could have shipped almost 500 bits at 57600, double that at 115200.  It wouldn't take but a couple dozen to say wait N millisec, write val X or wait N' millisec, read X', return data.

Could you just have a simple state machine running on the CC side that responds to the individual steps in the protocol as you go?!?  All the intelligence is on the GCS side of the link, and it drives the protocol by "remote control" by shipping over these command packets?   Crazy idea?

Like I said, just thinking out loud.

I had similar thoughts to just this. While for basic I2C use, I think it becomes difficult to ensure proper longterm operation, since the onboard state machine would effectively go dumb if there were comm problems, for more complex operation schemes this might actually be a very good way of handling things. For instance, one of the pressure sensors that was considered for OpenPilot uses a fairly complex pattern in the beginning in order to get calibration data, but then after that the actual reading of the chip itself is very straightforward. So imagine that the GCS handles this part, while leaving the looping sensor reading rest to the onboard code. I like that idea, and don't see why it should be difficult to pull off with sufficiently generic onboard code.

Of course, with the upcoming OP Revolution, RAM is no longer a problem so there's really no need to eek out every last byte. I really wish there were a clear processor upgrade path for the CC, but it doesn't seem as if STM is intent on making F2 and F4 chips in that package size.

P.S. I tried using kenz/genericI2CSensor, but it seems that loading the module causes the CC to fail to boot. If someone out there can take a look with a JTAG debugger, I'd appreciate the feedback on what's going wrong.

[Scott]

You may want to try removing some modules in the Makefile if the CC is failing to boot. I've had this problem and solved it via this method which frees up the maxed-out resource (ie. CPU or memory).