Linker error: using pulseIn() realized as puls_asm.S

[bojh]

Using the pulseIn() method in a sketch (e.g.)

#define PULSE_PIN 3
void setup() {
  Serial.begin(115200);
  pinMode(PULSE_PIN, INPUT_PULLUP);
  uint32_t pulse = pulseIn(PULSE_PIN, HIGH, 6000);
  Serial.print  ("pulse len[us]: ");
  Serial.println(pulse);  
}
void loop() {}

does create an linker error: see output log:

c:/users/.../appdata/local/arduino15/packages/sandeepmistry/tools/gcc-arm-none-eabi/5_2-2015q4/bin/../lib/gcc/arm-none-eabi/5.2.1/../../../../arm-none-eabi/bin/ld.exe: 
error: C:\Users\...\AppData\Local\Temp\build564232db8aa323b9aba1a2502c4097a0.tmp/test_pulseIn.ino.elf 
uses VFP register arguments, 
C:\Users\...\AppData\Local\Temp\build564232db8aa323b9aba1a2502c4097a0.tmp/core\core.a(pulse_asm.S.o) 
does not

c:/users/.../appdata/local/arduino15/packages/sandeepmistry/tools/gcc-arm-none-eabi/5_2-2015q4/bin/../lib/gcc/arm-none-eabi/5.2.1/../../../../arm-none-eabi/bin/ld.exe: 
failed to merge target specific data of file C:\Users\...\AppData\Local\Temp\build564232db8aa323b9aba1a2502c4097a0.tmp/core\core.a(pulse_asm.S.o)

In the assumption that the DUE configuration (platform.txt) has a proper setting, I have tried to find a difference but I'm not succesful up to now.

[bojh]

A step forward - but not final: I assume that the compiler flags for assembler code should be set in the same way as C,CPP and LD. This would impact changes in the current nRF5 platform.txt as follows:

compiler.S.flags=-c -g -x assembler-with-cpp {build.float_flags} with aditional {build.float_flags} ... but no success at the moment. Can anybody verify that assumtion or know the reason?

[sandeepmistry]

So nRF51 links fine for me, but nRF52 does not.

Maybe the .cpu line needs to change? https://github.com/sandeepmistry/arduino-nRF5/blob/master/cores/nRF5/pulse_asm.S#L62

[bojh]

@sandeepmistry I have tried that short fix in using:

    .cpu cortex-m4

but no quick success. As a work-arround I have re-used meanwhile the related C code in puls.c (see below). I expect the .fpu softvfp is not correct too. We should compile the c code like the gcc statement in the pulse_asm.S ASM file for the nRF52 cortex-m4 to generate the ASM source. The only difference and goal I can see, would be the -Os option.

// See pulse_asm.S
//TODO extern unsigned long countPulseASM(const volatile uint32_t *port, uint32_t bit, uint32_t stateMask, unsigned long maxloops);
    unsigned long countPulse_C(const volatile uint32_t *port, uint32_t bit, uint32_t stateMask, unsigned long maxloops)
    {
      unsigned long width = 0;

      // wait for any previous pulse to end
      while ((*port & bit) == stateMask)
        if (--maxloops == 0)
          return 0;

      // wait for the pulse to start
      while ((*port & bit) != stateMask)
        if (--maxloops == 0)
          return 0;

      // wait for the pulse to stop
      while ((*port & bit) == stateMask) {
        if (++width == maxloops)
          return 0;
      }
      return width;
    }

/* Measures the length (in microseconds) of a pulse on the pin; state is HIGH
 * or LOW, the type of pulse to measure.  Works on pulses from 2-3 microseconds
 * to 3 minutes in length, but must be called at least a few dozen microseconds
 * before the start of the pulse. */
uint32_t pulseIn(uint32_t pin, uint32_t state, uint32_t timeout)
{
  // cache the port and bit of the pin in order to speed up the
  // pulse width measuring loop and achieve finer resolution.  calling
  // digitalRead() instead yields much coarser resolution.
  // PinDescription p = g_APinDescription[pin];
  uint32_t bit = 1 << pin; //p.ulPin;
  uint32_t stateMask = state ? bit : 0;

  // convert the timeout from microseconds to a number of times through
  // the initial loop; it takes (roughly) 13 clock cycles per iteration.
  uint32_t maxloops = microsecondsToClockCycles(timeout) / 13;

  uint32_t width = countPulse_C(&(NRF_GPIO->IN), bit, stateMask, maxloops);
//TODO  uint32_t width = countPulseASM(&(NRF_GPIO->IN), bit, stateMask, maxloops);

  // convert the reading to microseconds. The loop has been determined
  // to be 13 clock cycles long and have about 16 clocks between the edge
  // and the start of the loop. There will be some error introduced by
  // the interrupt handlers.
  if (width)
    return clockCyclesToMicroseconds(width * 13 + 16);
  else
    return 0;
}

[rogerclarkmelbourne]

You could try using inline assembler in the C file

We had a discussion about this on the STM32Duino forum

http://www.stm32duino.com/viewtopic.php?t=1177

e.g.

/* Example of calling an assmebler function from C, based on
 *  code examples by Harald Kipp
 *  http://www.ethernut.de/en/documents/arm-inline-asm.html
 */
unsigned long ByteSwap(unsigned long val)
{
asm volatile (
        "eor     r3, %1, %1, ror #16\n\t"
        "bic     r3, r3, #0x00FF0000\n\t"
        "mov     %0, %1, ror #8\n\t"
        "eor     %0, %0, r3, lsr #8"
        : "=r" (val)
        : "0"(val)
        : "r3"
);
return val;
}

void setup() {
  // put your setup code here, to run once:
  Serial.begin(115200);
}

void loop() {
  // put your main code here, to run repeatedly:
Serial.println(ByteSwap((unsigned long)0x12345678),HEX);
delay(1000);
}

[bojh]

@rogerclarkmelbourne good to know, but ASM will always be platform dependend. I'm just not be able to create the right ASM code for that and I can not judge the exact benefit of this specific ASM implementation - but there was a orginally intention.

[rogerclarkmelbourne]

What about using the Due's code

https://github.com/arduino/Arduino/blob/15df77a000019c7833f8cb8cbae6c6ba533d12ef/hardware/arduino/avr/cores/arduino/wiring_pulse.S

I know the Due has a FPU, but I can't tell by looking in the code whether the assembler uses those instructions

Or RBL's

https://github.com/RedBearLab/nRF51822-Arduino/blob/3fd208e1e19d9fc56eadb4f8cefa28f59e44a14d/arduino-1.6.x/hardware/RBL/RBL_nRF51822/cores/RBL_nRF51822/wiring_shift_pulseIn.cpp

However, I'd probably bypass the digitalRead and go straight to the hardware

[bojh]

@rogerclarkmelbourne DUE has an avr-gcc. I thing this more away. RBL for the nRF51822 does it also with the C source and that has worked for me in the past. So my voting would be

1. take the C- code like upper comment
2. at free time we can try to compile C source with ASM generation like described in the pulse_asm.S source for the cortex-m4

[sandeepmistry]

@bojh I'd be open to using the C code on nRF52 for now, however for nRF51 let's continue to use the ASM. Please submit a pull request for this.

[sandeepmistry]

@bojh ping ... can you submit a PR for this?

[bojh]

I do not have used this function in my application anymore. This morning I have given a chance again - but I haven't got my proposal (see above) working at the moment - don't know why (always got an width value=0) - so we have to dive deeper - any idea?. See attached testing source: test_pulsIn.zip