PDA

View Full Version : Is there a limit to local longs? Strange behavior when I initialize some local longs.



turbosupra
03-14-2012, 03:26 PM
I have code with a bunch of local longs and I was testing to see if there was a limit to that.

When I do not comment out some of the initializations of the local longs, it appears that it hoses the repeat loop or maybe its timing.

When I comment out lines 189-197, the loop then works. Can anyone please explain why this is occurring for my own learning? I have pin 4 feeding pin 5 through a 100ohm resistor.

I can't attach the code to the forum because of the work firewall, but I can place it here. As a warning, the code is not cleaned up, things are commented out everywhere and normally I would try and clean it up a bit, but since this is such an oddball issue, I wanted to present it as is.


http://turbosupra.herobo.com/hosting/seq.zip

Thanks for reading.

Duane Degn
03-14-2012, 03:33 PM
I haven't looked at the code yet, but do you know local longs need to be initialized to zero? They will start with some random number if not initialized. Only "result" starts out zero.

Duane Degn
03-14-2012, 03:35 PM
I'm getting a directory error when I try to extract the file.


Caution: Zip file comment truncated.
Error: central directory not found.

MagIO2
03-14-2012, 03:49 PM
Local longs are placed on the stack. So, if your stack is to small you should expect any kind of problems.
Potentially there might also be a limit, as SPIN might use a byte offset for referencing local variables?! Not sure if I remember having heared something like that ;o)

But in general it's a bad idea to code the propeller in the same way that you'd code a PC. Using to many local variables should simply be avoided.

Can't access your ZIP file because of firewall settings here. Your hoster seems to be on our "NoNo"-list ;o)

@Duane:
Why do they have to be initialized to zero? What if zero does not make sense? Hopefully it can be reassigned to other values after initialization! ;oP

Duane Degn
03-14-2012, 03:54 PM
Using to many local variables should simply be avoided.


I agree. They also don't show up as using the correct amount of memory when you press "F8".

idbruce
03-14-2012, 03:54 PM
I haven't looked at the code yet, but do you know local longs need to be initialized to zero? They will start with some random number if not initialized. Only "result" starts out zero.


I did not know that. Now I will have to recheck all my source for errors. What a bummer.

MagIO2
03-14-2012, 04:02 PM
I did not know that. Now I will have to recheck all my source for errors. What a bummer.

I think that's common to most kind of stacks. It's only that other compilers will give you at least a warning that your variable has not been initialized. But I think this is a kind of a problem you would not be aware of even if it's describes somewhere in the manual. Some things you have to find out the hard way ;o)

Duane Degn
03-14-2012, 04:02 PM
@Duane:
Why do they have to be initialized to zero? What if zero does not make sense? Hopefully it can be reassigned to other values after initialization! ;oP

;oP ;oP ;oP :smile:
As you know, when the "not initilized" thing is a problem it's because the programmer is expecting the values to equal zero as global variables do.

And yes, they can be initialized to anything you want (as long as it fits in a long). Germans!! :smile:

I think I should be the only one allowed to purposefully misinterpret a post.

idbruce
03-14-2012, 04:08 PM
MagIO2

Before making my post, I checked the Propeller Manual for PUB, and sure enough it stated that local variables had a random value.

I normally initialize locals with values, but I may have taken it for granted in a few instances.

Bruce

idbruce
03-14-2012, 04:14 PM
Duane

I am not sure whether to thank you or curse you for that info. :)

I think I will thank you.

Bruce

turbosupra
03-14-2012, 04:29 PM
Eh ... I'm getting that error too now.

Can I email it to you instead?



I'm getting a directory error when I try to extract the file.


Caution: Zip file comment truncated.
Error: central directory not found.

Duane Degn
03-14-2012, 04:44 PM
Eh ... I'm getting that error too now.

Can I email it to you instead?

You can and then I'll post it for you.

My email is my first initial followed by my last name (five letters total so far) at abreviation for Micro Soft Network dot com.

turbosupra
03-14-2012, 05:00 PM
Thanks everyone for the replies.

I have increased the stack size of the other cog calls and because they weren't related to the main cog that didn't make a difference I guess, could it be local cog ram related?

Also, this is in the main cog, so is there a way to hard code the amount of stack size for that?

Rayman
03-14-2012, 05:08 PM
I thought you figured out is was due to the local variables not being initialized...

Duane Degn
03-14-2012, 05:33 PM
Thanks everyone for the replies.

I have increased the stack size of the other cog calls and because they weren't related to the main cog that didn't make a difference I guess, could it be local cog ram related?

Also, this is in the main cog, so is there a way to hard code the amount of stack size for that?

The cog ram only has the Spin interpreter in it (unless you're running PASM code). The interpreter runs the code from hub and sets aside "stack" space for that cog in the hub (how could that be confusing?:smile:).

turbosupra
03-14-2012, 05:48 PM
Hi Rayman,

I was initializing all local variables originally. When I started to comment out some of those initializations in "main" before entering the repeat loop in "main", the code started working??? (backwards I know) I spent 6 hours messing around with it yesterday before I even got to that point of figuring that out. Since this is so bassackwards, I had to post about it and find out for my own sanity why this was happening and to avoid it.

It may be because I have so many local variables and if there is a ceiling on that, I would just like to know?



I thought you figured out is was due to the local variables not being initialized...

turbosupra
03-14-2012, 05:49 PM
Ok, so it isn't cog ram :) ... could it be the initial cogs stack and that I've exceed the space for that with local variables?

Did you receive my email? I guessed at the abbreviation as msn.com ?


The cog ram only has the Spin interpreter in it (unless you're running PASM code). The interpreter runs the code from hub and sets aside "stack" space for that cog in the hub (how could that be confusing?:smile:).

Rayman
03-14-2012, 05:55 PM
Ok, it sounds like you are out of stack space then. Your stack space is overlapping with your code. So, when you initialized it, you erased your code...

Duane Degn
03-14-2012, 06:08 PM
Did you receive my email?

Sorry, day job got in the way.

It will be a while before I can look at it.

turbosupra
03-14-2012, 06:10 PM
Ok, that makes sense Rayman. Is there a way to manually set the main cog's stack, so that I can verify this before I covert the local variables to global variables?

turbosupra
03-14-2012, 06:13 PM
No worries, just wanted to make sure I got the address correct.

Thanks for attaching.




Sorry, day job got in the way.

It will be a while before I can look at it.

Duane Degn
03-14-2012, 06:16 PM
I notice the Prop Tool doessn't like to display local variables it they extend past the intial screen.

You might want to break them into several lines like this.


PUB Main | localIndex, localPeriod, localCamPeriod, localCrankPeriod, timeStamp, {
} currentCam, previousCam1, previousCam2, previousCam3, previousCam4, {
} previousCam5, previousCam6, trigger1, trigger2, trigger3, currentCrank, {
} previousCrank1, previousCrank2, previousCrank3, previousCrank4, {
} previousCrank5, previousCrank6, previous5CrankAverage, elapsedTime, {
} previousGap, currentGap, toothGap, toothGap1, toothGap2, toothGap3, {
} toothGap4, toothGap5, toothGap6, previousToothGapAverage, toothGapRpm, {
} crankToothRpm, previousNum1, previousNum2, previousNum3, previousNum4, {
} previousNum5, previousNum6, current, previous5average, toothRpm

turbosupra
03-14-2012, 06:24 PM
^ Sweet, I didn't know you could do that, that is a great trick!

The problem still exists after doing that, but that makes readability much much easier.

turbosupra
03-14-2012, 07:01 PM
I just converted them all to global variables and had the same problem, then I converted them all back to local variables, still had the same problem ... commented some of the local variables out and it worked as expected?

So although there may be a local variable limit based on the main cogs stack, I don't believe that was my issue after this testing procedures results

The mystery continues ...

Rayman
03-14-2012, 07:08 PM
Let me ask some usual questions... How my free longs does the compiler say you have? Are you using Graphics.spin?

turbosupra
03-14-2012, 07:22 PM
Hi,

Program - 1023 longs
Variable - 437 longs
Stack/Free - 6728 longs

I am not using graphics.spin

I am using



OBJ
pst : "Parallax Serial Terminal"
n : "Numbers"
fcam : "jm_freqin"
fcrank : "jm_freqin"
f : "jm_freqin"




Let me ask some usual questions... How my free longs does the compiler say you have? Are you using Graphics.spin?

Rayman
03-14-2012, 08:12 PM
You have plenty of free space then... I'd guess you have an error in your code and not a memory issue...

Unless... Are you calling that routine with a lot of local vars recursively?

MagIO2
03-14-2012, 08:31 PM
The main function running in the initial COG has the whole free HUB-RAM as stack-space. That's what this "Stack/Free - 6728 longs" tries to tell you ;o)

If these things happen when commenting code you maybe have a problem with race conditions. For example several code-pieces running in different COGs use the same variable one has to write the other reads and does calculations with the read value. In these circumstances such problems might occur if the writer code did not run in time.
It could also be related to the cognew which needs some time to start up the code.

Rayman
03-14-2012, 08:34 PM
Sometimes Stack/Free lies though... A lot of people start from Graphics_Demo, which uses a huge amount of undeclared HUB ram at the top of memory...

turbosupra
03-14-2012, 08:40 PM
The psuedocode is

Cog sends frequency out pin 4, jm_freqin reads frequency on pin 5, main cog polls jm_freqin.period method every so often for value and calculates data

This could be the race condition you describe, but would it continually loop over and over, or would it only 0 out in some cases?

Do I need to copy the value before performing calculations on it? How do you eliminate the possibility of a race condition?



The main function running in the initial COG has the whole free HUB-RAM as stack-space. That's what this "Stack/Free - 6728 longs" tries to tell you ;o)

If these things happen when commenting code you maybe have a problem with race conditions. For example several code-pieces running in different COGs use the same variable one has to write the other reads and does calculations with the read value. In these circumstances such problems might occur if the writer code did not run in time.
It could also be related to the cognew which needs some time to start up the code.

MagIO2
03-14-2012, 11:03 PM
The things going on here are pretty complex. I've spotted the following:

1. You seem to work with timings in several areas of your program. But on the other hand you are very generous with usage of PST. But PST has a buffer of 64 bytes. So, when you extend the number of characters send this will slow down your whole program and the conversions will be done in the COG calling the PST functions anyway. So, I'd suggest to think about a debugger/logger COG which runs independently.

2. Chances are high, that these kind of if-statements don't do what you expect
if(cnt > (timeStamp + (clkfreq/l_updateRateDenominator)))

The problem is that the > operation is working with signed numbers. So, from $0000_0000 - $7fff_ffff it will see a positive number and from $ffff_ffff-$8000_0000 it will see negative numbers. In other words for the > operator $0000_0000 will be bigger than $8000_0000.
If you work with a difference you don't have this problem:
if((cnt-timeStamp) > (clkfreq/l_updateRateDenominator))

turbosupra
03-14-2012, 11:27 PM
Hi,

Thanks for taking the time to look through the code and for the reply, I have been in this situation before and "coding around" it never fixes the problem permanently.

The main cog was originally going to be the debugger cog, since I have jm_freqin using 2 cogs and the simulation cogs sending to both jm_freqin objects/cogs using another 2 cogs. Do you think I should have a separate cog outside of the main cog for debugging? Normally I'm only sending a few lines of pst, as I understand what you are saying and how it slows things down, although I know in the unclean version of the code I posted there was commented out pst code everywhere.

As for number 2, I appreciate you pointing that out. I have not seen an issue from that yet, but I'm sure it would have come up. I will make that change tonight. I will also being putting the 2 calculation parts that are in the main loop, into methods and calling them from the main loop.

Let me know if I should allocate a separate cog entirely from the main cog for debug data and if you have any other "best practice" suggestions.

Thanks again! :)





The things going on here are pretty complex. I've spotted the following:

1. You seem to work with timings in several areas of your program. But on the other hand you are very generous with usage of PST. But PST has a buffer of 64 bytes. So, when you extend the number of characters send this will slow down your whole program and the conversions will be done in the COG calling the PST functions anyway. So, I'd suggest to think about a debugger/logger COG which runs independently.

2. Chances are high, that these kind of if-statements don't do what you expect
if(cnt > (timeStamp + (clkfreq/l_updateRateDenominator)))

The problem is that the > operation is working with signed numbers. So, from $0000_0000 - $7fff_ffff it will see a positive number and from $ffff_ffff-$8000_0000 it will see negative numbers. In other words for the > operator $0000_0000 will be bigger than $8000_0000.
If you work with a difference you don't have this problem:
if((cnt-timeStamp) > (clkfreq/l_updateRateDenominator))

MagIO2
03-15-2012, 08:28 AM
What I would do .... well ... I'd put together things that belong together.

For example create an object for that crank thing (sorry, but the following code is untested again - no PropTool available currently):


VAR
byte pin, presentTeeth, missingTeeth, cogID
long frequencyRate

long pulse, sync, timing, localIndex
long stack[10]

PUB start(p, initFrequency, pt, mt)
pin := p
presentTeeth := pt
missingTeeth := mt

if cogID
cogstop( cogID-1 )

cogID := cognew( crankPulser, @stack )+1

PUB setFrequency( newFrequ )
if( newFrequ <= 0 )
frequencyRate := 7000
else
frequencyRate := newFrequ <# 7000

timing := clkfreq / frequencyRate

PUB getFrequency
return frequencyRate

PRI crankPulser

'' Output 1 to 500Hz signal on pin
'' -- high-going pulse is fixed to 1ms
'' -- pntr is hub address of signal frequency (1 to 500Hz)

ctra := (%00100 << 26) | pin ' setup counter for pin
frqa := 1
phsa := 0

dira[pin] := 1 ' make output

pulse := clkfreq / 10_000 ' ticks in 1ms

sync := cnt ' sync with system clock
repeat

repeat localIndex from 1 to (presentTeeth- missingTeeth)
phsa := -pulse ' create 1ms pulse
waitcnt(sync += timing) ' hold for current hz setting

repeat localIndex from 1 to (missingTeeth)
'phsa := -pulse ' create 1ms pulse
waitcnt(sync += timing)


The point is that the number of locals has been shrinked dramatically. The compiler now has a chance to complain if the data size + code size extends the available RAM. Using local variables will not complain even if your stack is to small and can produce any kind of strange and very hard to find bugs!

You have this in a separate file now and can easily setup a small program which only tests this part. The test program also gives you a good impression on how to use the object without any code that's dealing with completely different stuff - like in your real project main program. This is very usefull if coming back to the program a few years later ;o)

Moving the variables to the VAR section of the object has one drawback: these variables are not accessible by the program using the object. So you have to add getter and setter functions as needed. But on the other hand the setter function can already do the validation and makes sure that only valid values are written into the variables.

Now it's also very easy to create 2 instances running with different frequencies on different PINs:

OBJ
pulser1: "NewPulserCode.spin"
pulser2: "NewPulserCode.spin"

or

pulserArray[2]: "NewPulserCode.spin"

and let it run:
pulser1.start( 4, 6000, 36, 2 )
pulser2.start( 16, 500, 36, 2 )

and change the frequency
pulser1.setFrequency( 3000 )


Run a new COG for debugging ... my general answer would be YES.
It's only needed if you deal with timings because sending debug information to PST might destroy your timings if you do it in the program itself. It can happen that it does not work because of debugging - it can also happen that it works with debugging, but without it no longer works!

Adding debug statements to several COGs is also problematic!

Having simple blinking LEDs that show the status is sometimes also very helpfull and is much faster than sending strings and converted numbers via serial interface.

pedward
03-15-2012, 09:15 AM
MagIO2

Before making my post, I checked the Propeller Manual for PUB, and sure enough it stated that local variables had a random value.

I normally initialize locals with values, but I may have taken it for granted in a few instances.

Bruce

I've learned through years of programming that you never assume any variable is initialized, until it is! ;)

idbruce
03-15-2012, 10:55 AM
pedward


I've learned through years of programming that you never assume any variable is initialized, until it is! ;)

I agree and I actually know better than to assume anything.

Bruce

turbosupra
03-15-2012, 06:32 PM
I think I found my mysterious issue. I'm trying to implement the changes you recommended and have a question.

When I do not have some local variables in main, one of my functions does not work. Can you tell me what this means? What does it mean when you put long[] in front of a pointer? pntr is a pointer to an @long variable in my code.


timing := long[pntr]

There is some sort of timing issue with my code I believe. If it just tells the code to read the long value, why would it be where you pass the address to the method parameter, and then read the long value of the address that the method parameter stores? Why not just pass the long to begin with?



What I would do .... well ... I'd put together things that belong together.

For example create an object for that crank thing (sorry, but the following code is untested again - no PropTool available currently):


VAR
byte pin, presentTeeth, missingTeeth, cogID
long frequencyRate

long pulse, sync, timing, localIndex
long stack[10]

PUB start(p, initFrequency, pt, mt)
pin := p
presentTeeth := pt
missingTeeth := mt

if cogID
cogstop( cogID-1 )

cogID := cognew( crankPulser, @stack )+1

PUB setFrequency( newFrequ )
if( newFrequ <= 0 )
frequencyRate := 7000
else
frequencyRate := newFrequ <# 7000

timing := clkfreq / frequencyRate

PUB getFrequency
return frequencyRate

PRI crankPulser

'' Output 1 to 500Hz signal on pin
'' -- high-going pulse is fixed to 1ms
'' -- pntr is hub address of signal frequency (1 to 500Hz)

ctra := (%00100 << 26) | pin ' setup counter for pin
frqa := 1
phsa := 0

dira[pin] := 1 ' make output

pulse := clkfreq / 10_000 ' ticks in 1ms

sync := cnt ' sync with system clock
repeat

repeat localIndex from 1 to (presentTeeth- missingTeeth)
phsa := -pulse ' create 1ms pulse
waitcnt(sync += timing) ' hold for current hz setting

repeat localIndex from 1 to (missingTeeth)
'phsa := -pulse ' create 1ms pulse
waitcnt(sync += timing)


The point is that the number of locals has been shrinked dramatically. The compiler now has a chance to complain if the data size + code size extends the available RAM. Using local variables will not complain even if your stack is to small and can produce any kind of strange and very hard to find bugs!

You have this in a separate file now and can easily setup a small program which only tests this part. The test program also gives you a good impression on how to use the object without any code that's dealing with completely different stuff - like in your real project main program. This is very usefull if coming back to the program a few years later ;o)

Moving the variables to the VAR section of the object has one drawback: these variables are not accessible by the program using the object. So you have to add getter and setter functions as needed. But on the other hand the setter function can already do the validation and makes sure that only valid values are written into the variables.

Now it's also very easy to create 2 instances running with different frequencies on different PINs:

OBJ
pulser1: "NewPulserCode.spin"
pulser2: "NewPulserCode.spin"

or

pulserArray[2]: "NewPulserCode.spin"

and let it run:
pulser1.start( 4, 6000, 36, 2 )
pulser2.start( 16, 500, 36, 2 )

and change the frequency
pulser1.setFrequency( 3000 )


Run a new COG for debugging ... my general answer would be YES.
It's only needed if you deal with timings because sending debug information to PST might destroy your timings if you do it in the program itself. It can happen that it does not work because of debugging - it can also happen that it works with debugging, but without it no longer works!

Adding debug statements to several COGs is also problematic!

Having simple blinking LEDs that show the status is sometimes also very helpfull and is much faster than sending strings and converted numbers via serial interface.

MagIO2
03-15-2012, 09:34 PM
someadr := @value

timer := long[ someadr ]
is the same as
timer := value

The point is that in function calls the parameters are passed by value. Something like


VAR
long frequency

PUB main
frequency := 50

cognew( doSomething( frequency ), @stack )
...

PRI doSomething( f )
...

means: start a new COG and execute function doSomething by passing 50. You will find 50 in the local variable f. There is no link between frequency and f. When you change frequency in the main, f still has the value 50. By passing the address you actually have a link to frequency in doSomething and you can always read the actual value of frequency.

About your problem of not having a local variable: that's why there is a getter-function in the object. If you need the value of frequencyRate you'd simply call the getFrequency function.

By the way I found a bug in my example: in the start function initFrequency is not stored. Simply calling setFrequency solves the problem.

turbosupra
03-16-2012, 04:35 AM
Thank you, I thought that was what that did but wanted to verify and not assume. Thank you for all you've shown me :)

I'm slowly trying to migrate to your code format.

I have a cog operating the com data now, but I'm not sure if I'll be able to use crankPulser because I was seeing some inconsistencies that I didn't understand.

Here is my current code, it works ok, but I cannot get one of the calculated values if I increase the line below to 10000, at 5000 it does fine.


cognew(simulateCrankWheel(4, 36, 2, 5000), @simulateCrankWheelStack)


I'm just not sure why the code below fails to read the low pulse timing gaps that will appear after each set of 34 successive "high/low" pulses, I don't think it's a speed issue but certainly appears that way??? Any thoughts as to what my mistake is here?


localCrankPeriod := fcrank.period

if(localCrankPeriod) <> 0 and l_pstReadCrankWheel == 1
calculateCrankRpm(localCrankPeriod)

turbosupra
03-16-2012, 04:55 PM
Well my problem definitely appears to be a timing issue, I think the variable is being written to when I'm trying to read it, so in the old cog setup, I had the calculations, the writing to and the read all being done in the same thread making it impossible for the variable to be written to while it was being read, because it was all in one process sequentially.

Now that I have separate threads, above a certain speed I cannot reliably read and write to the same long. How to fix this, I have no idea :) but this at least gives me some closure on the inconsistencies.

MagIO2
03-16-2012, 10:03 PM
I think your timings don't fit. On one hand you allow a value of 5000 as frequency setting but your pulse should be 1ms. 1ms gives you a max frequency of 1000Hz.

Sometimes it's better to start from scratch again. Maybe you want to continue with my version. It uses the crank object I already posted before. Please have a closer look at the settings, as I used a LED-pin of my quickstart-board as output, so I can see what happens.
I also reduced the baudrate of PST ... don't know why, but it's my preferred setting ;o)

Such kind of parameters should rather be defined in the CON section at the very beginning, so they can easily be changed without changing something inside of the code.

Most important change was the setup of the counter. Your setup used PLLDIV of Vco / 128 which means that the counter runs much slower than the clock, but calculations are based on clkfreq.

What I do very often in terminal based projects is to wait until a key is pressed before really starting the program.

turbosupra
03-16-2012, 10:37 PM
I will try with your code, I certainly appreciate the time you spent writing it.

Maybe I'll take a night off, I've been trying to get this working for 2 weeks now and I may need some clarity by taking a step back. I see what you mean about the counter now, that was due to my ignorance, I was modifying what someone else wrote and need higher than a 500hZ resolution. I'll try this and report back my results.

Thanks again :)

MagIO2
03-16-2012, 10:43 PM
Oh ... and your
PUB elapsed_time_msec(p_TimeVar) | RightNow, ClockTicks
is to complicated!

If you do
currentTime - startTime
your calculation would be right even if you had a wraparound. No need for this:


if RightNow < p_TimeVar
ClockTicks := ||($FFFFFFFF - RightNow + p_TimeVar)
else
ClockTicks := ||(RightNow - p_TimeVar)


just say

ClockTicks := RightNow - p_TimeVar

kuroneko
03-17-2012, 01:24 AM
Most important change was the setup of the counter. Your setup used PLLDIV of Vco / 128 which means that the counter runs much slower than the clock, but calculations are based on clkfreq.
Why would an NCO (%00100) be affected by the PLLDIV field?

turbosupra
03-17-2012, 01:26 PM
I would like to know that and what would make NCO at better or worse choice for this, than say POS (%01000)?

kuroneko
03-17-2012, 01:41 PM
... what would make NCO at better or worse choice for this, than say POS (%01000)?
AFAIK you want to generate a pulse train which means you're limited to PLL/NCO/DUTY modes. All other modes don't produce outputs (not counting feedback which is just the inverted input).

MagIO2
03-17-2012, 05:00 PM
Thanks kuroneko!

I did not use counters too often so far!
In retrospective:
I used turbosupras code with the counter connected to a quickstart LED. What I saw was that the LED in fact was blinking, but veeery slow. That's why I thought the counter would maybe run with the wrong frequency. And I changed it without a re-read of the manual.
At the same time I spotted the problem with the frequency setting and changed that as well. Looks like the real fix was the frequency setting.

@turbosupra:
Did you run the code? Is that what you want crank to do?
If you want to go higher than 500Hz, you can't live with an pulse width of 1ms. Simply change this as well and you can go higher.

MagIO2
03-17-2012, 07:42 PM
@turbosupra:
Simply remove the calculation of pulse from crankPulser and put it into setFrequency like that:

pulse := timing >> 1

Additionally you can define a MAX_FREQUENCY constant and use that in setFrequency as well.

turbosupra
03-18-2012, 04:51 AM
Thank you so much. It does simulate a crank trigger wheel and crank trigger sensor quite well according to the propscope.

My method that uses the jm_freqin object is even able to calculate rpm from it :) . I tried to write code that would count how many times a second, the cog was loop through the part of the code directly below, but I cannot get it to work?


repeat localIndex from 1 to (missingTeeth)
phsa := 0 ' create 1ms pulse
waitcnt(sync += timing)




I also noticed that my code would consider a "second" (as defined below) every 81600000 cycles instead of 80000000 which was also strange. I initially define timeStampCrank once the first time the method is called and then I try and enter the part of the code underneath of this, when cnt-l_timeStampCrank is greater than 80000000 clk cycles. I cannot for the life of me figure out why the timing would be so far off at 81.6 million cycles? Am I approaching this incorrectly?


if (l_timeStampCrank == 0)
l_timeStampCrank := (cnt) if((cnt-l_timeStampCrank) > clkfreq)





Thanks kuroneko!

I did not use counters too often so far!
In retrospective:
I used turbosupras code with the counter connected to a quickstart LED. What I saw was that the LED in fact was blinking, but veeery slow. That's why I thought the counter would maybe run with the wrong frequency. And I changed it without a re-read of the manual.
At the same time I spotted the problem with the frequency setting and changed that as well. Looks like the real fix was the frequency setting.

@turbosupra:
Did you run the code? Is that what you want crank to do?
If you want to go higher than 500Hz, you can't live with an pulse width of 1ms. Simply change this as well and you can go higher.


@turbosupra:
Simply remove the calculation of pulse from crankPulser and put it into setFrequency like that:

pulse := timing >> 1

Additionally you can define a MAX_FREQUENCY constant and use that in setFrequency as well.

MagIO2
03-18-2012, 12:20 PM
I don't know why you want to measure the time for this loop! The time this loop needs is pre-defined by the timing variable.

sync := cnt
....
waitcnt( sync += timing )

goes on with execution exactly after timing clockticks, except you missed the cnt-value which adds ~53seconds to the runtime. So, in your loop you maybe have to add some cycles for the condition-check of the repeat .. from, but this should be some hundred clocks.

Well ... with the cnt - l_timestampCrank - method you don't wait for an exact time. You want to continue in your program and start something if the time has exceeded. How much it can exceed of course depends on the other code running in that loop. If you have some serial output this might easily extend 1/10th of a second. Makes sense to post the whole code here to get an idea which code delays the loop.

turbosupra
03-18-2012, 11:48 PM
Hi,

The reason I want to measure time, is because I want to calculate how many times the following code is passed through each second, so # of times/sec.


repeat localIndex from 1 to (missingTeeth)
phsa := 0 ' create 1ms pulse
waitcnt(sync += timing)

I am not doing any pst writing in that cog, (it is called in the main cog) as I learned my lesson on that one :)

I've attached an archive of the code, thank you for being willing to look at it.

MagIO2
03-19-2012, 11:39 AM
Again ... you are the one who defines how long to wait in the waitcnt! I currently can't see a good reason to MEASURE what you already know! Don't you trust your code?

clkfreq / (presentTeeth * timing ) gives you the answer. (Assuming you can live with integers)

Ok, no PST calls in between, but you still do a lot in between two calls of calculateCrankRpm. So, let's say you call calculateCrankRpm and the code reaches the if statement when cnt - l_timeStampCrank is equal to 79_999_999. Your code has to go another round:
1. return from calculateCrankRpm
2. call fcrank.freq - doing calculations
3. call fcrank.period
4. call calculateCrankRpm again which first runs through a lot of calculations
Pretty sure this can sum up to 1600000 additional clock cycles

What's the purpose of calculateCrankRpm? Looks like it's doing some averaging. Maybe you want to have a look into this thread http://forums.parallax.com/showthread.php?137868-Moving-average-question-from-a-Beau-Schwabe-reply

It looks like you could simply add a function getRPM into Crank.spin, which simply returns
clkfreq / (presentTeeth * timing ) * 60

I don't know enough about your problem domain, so I can't easily estimate what the purpose of the code is. Maybe you can explain the next step and we can develop your application step by step.

turbosupra
03-19-2012, 02:02 PM
I wanted to measure Rpm the second way because I wanted some redundancy, but this may be me being over cautious and may not be needed.

calculateCrankRpm allows me to take a rolling average of how many clk cycles it took to go from positive edge to positive edge, for the last 5 teeth. It also assumes that if the clk cycle count is bigger than the previous average multiplied by 1.1, that it is the missing teeth gap (which may also be a long solid tooth instead of a long gap, I have to verify this by disassembling something but it serves the exact same purpose either way), and it does not include that value in its rolling average. That thread was a good read.

If I added clkfreq / (presentTeeth * timing ) * 60, I could keep track of presentTeeth the way I keep track of l_Cnt, but I'm fuzzy on keeping track of the time I've done since last calculation (timing). The reason being, is that I should get l_Cnt to go to ~167 at 10000rpm, because this means the crank is rotating 167 times a second, and you get +1 to l_Cnt for each single full rotation. I'm not seeing anywhere near that, even when I go 81.6 million cycles, or about 1.02 seconds.

To help illustrate, here is an idea of what I my hall effect sensor trigger wheels look like, the "sensing" part is done by a hall effect sensor. My goal is to keep track of how fast I'm going from the positive edge of one tooth to the positive edge of another tooth and calculate rpm from it. I also want to be able to reference which tooth I am at in the rotation. Does that make sense? This sensor is the main component of a car/truck in telling the cars computer when to fire a fuel injector to a certain cylinder and when to fire a spark plug for a certain cylinder.

http://img714.imageshack.us/img714/760/trigger004.jpg

turbosupra
03-19-2012, 07:58 PM
I made a few small code changes just now by adding some flags and the first time fully through the code you get 167 (l_CntPeak) for the 80,443,840 clk cycles, which is right within my numerical expectation. 80,443,840/80,000,000 = 1.005548 and 167/1.005548 = 166.0786 . After that it will read between 163 and 166, but it should be consistent I would think? Is my logic on or off with this?

Is there a standard way to time how long a method takes, or is my l_timeStampSubtracted sufficient?





Starting Crank COG!
frequency set to 6000
9998
0
0
0
9992


0
10001
0
0
0
0


0
9998
80443840
167
0
0
0


0
10001
80443840
0
0
0


0
10000
80157984
163
0
0
0


0
10000
80157984
0
0
0


0
9998
80157984
163
13344
10003
9992

MagIO2
03-19-2012, 09:47 PM
Hi Turbosupra ... I have some little changes - not functional changes because I'm currently a bit to confused!

First of all thanks for the nice picture. But that's exactly what confuses me. With the code you write .. do you want to simulate these trigger wheels for external usage or do you want to read/measure the sensor signals produced by these wheels and the current crank-code is only for checking the measure-code?

I don't really remember where the code (cnt - lastMeasuredCnt) > timespan came from - was it in your original post. I remember that I corrected your first implementation ... however .... this kind of measurement is good for tasks that you want to perform somewhen around that given timespan. Or more precicely in the range of exactly this timespan or more. This kind of code is not good if you need exact results.


l_Cnt += 1
if((cnt-l_timeStampCrank) > clkfreq)
l_timeStampCrankPrevious := l_timeStampCrank
l_timeStampCrank := cnt

Allowing a deviation in the if statement (in time) means in the end that the numbers might also vary from one to the next try.

If you really need this measurement-stuff, I'd suggest to add the moving average as outlined by Beau in the thread I mentioned into the PASM-part of Johns frequency measurement object. There you should have plenty of time for summing up some values.

MagIO2
03-19-2012, 10:01 PM
Ups .. forgot to post the changes ;o)

Only cosmetical changes showing you how I'd do the output.

Doing these changes I also got some new questions:
What does l_toothGapRpm mean?
What does l_toothRpm mean?
What is ((clkfreq/fcrank.period)*60)/36 ? Am I right with labeling it as RPM?
And finally why should the RPMs be different at all?
If the wheel turned one round each of it's teeth also turned one round and the gap turned one round?

turbosupra
03-19-2012, 10:43 PM
Hi!

I know that I am crossing two very different worlds with the mechanics and technical terms of automotive, and the electronics and technical terms of circuitry. I try to keep the automotive things out of the discussion to keep things less confusing, but if you have any questions about that I can explain that in as much detail if you would like.

As for the different methods. I have found that it is much easier troubleshooting and writing code from inside of my house, then outside in the car ... possibly even on the couch or in bed, so I decided to try and use the prop to simulate the sensors as best as possible, before I sat out in the car for hours in my driveway with the laptop, extension cord and wasting a lot of gasoline :D . Last summer, I did the opposite and wasted many gallons of gas, but I have now seen the beauty of the prop feeding itself signals and learned my lesson for this summer.

l_toothGapRpm is supposed to be the rpm calculated from the bridge or gap (the part I've circled in the picture below) . I believe the automotive manufacturers do this so that they can tell when an engine rotates past the tooth that they deem 0 or top dead center, but this is only speculation on my part. I do know however that it is there to differentiate when the 360 degree rotation starts.

l_toothRpm is supposed to be the rpm calculated from a single positive edge of one of those triggers, to the next positive edge of the following trigger. I have error checking in the code to throw out large variances such as the bridge/gap circled because the value would be way way off.

((clkfreq/fcrank.period)*60)/36 is rpm. It takes the amount of clk cycles between a rising edge and the previous rising edge before it and it then divides the clkfreq by that number as the denominator to calculate how many times a second that "period" would occur based on that single snapshot in time. Then it calculates the number of seconds in a minute, since Rpm is per minute and finally it divides by the number of times that "period" occurs before a single revolution of the trigger wheel has occurred. That sounded confusing when I wrote it, so I can explain better if it reads that way also.

I can't use straight frequency though, because of the delimiter that the automotive company has built into the hall effect trigger wheel, so I have to have logic that addresses that, which is why I have to take and average and insure that the new value is not outside of that average by a certain amount.


Ah, I did not know you could do that with the print screen, that is AWESOMEEEE! That's the coolest thing I have learned all day, does it by chance somehow save any overhead with bits? I like the constants too, that makes sense.

http://img809.imageshack.us/img809/6968/362triggerwheel1.jpg



Ups .. forgot to post the changes ;o)

Only cosmetical changes showing you how I'd do the output.

Doing these changes I also got some new questions:
What does l_toothGapRpm mean?
What does l_toothRpm mean?
What is ((clkfreq/fcrank.period)*60)/36 ? Am I right with labeling it as RPM?
And finally why should the RPMs be different at all?
If the wheel turned one round each of it's teeth also turned one round and the gap turned one round?

MagIO2
03-19-2012, 11:11 PM
"does it by chance somehow save any overhead with bits?"
I'm not sure wheather I understand this question correctly! You mean: Does it save bandwidth? ?
Currently it needs more bandwidth, but as you only update the screen once per second that's currently not an issue. The point is that this way the positioning information has to be send first and that old values need to be overwritten completely. For example if you output 83444 first and then the number decreases to 11 you have to overwrite 444 with spaces or you'd see 11444.
But some extra code could be added, which only updates the values that really changed. Depending on the stability of the values you might save bandwidth.

Ok ... so measuring is the essence of your program. So I'll concentrate on that tomorrow. Which max. RPM do we talk about?

turbosupra
03-20-2012, 03:06 AM
Yes, I was wondering if it saved bandwidth.

I don't believe I'd ever eclipse 10000rpm, so that would be a good ceiling.

MagIO2
03-20-2012, 08:41 AM
Simulating the wheel and it's signals is of course a good idea for producing some code without being attached to the real hardware. The only problem could be to write some code for a clean digital signal without dropouts and spikes when the real signal later on is not that clean. How good is the quality of the real signal later on?

Is this device you build for display-only or is it meant to control other hardware related to the input?
With the statement you gave earlier that you want to know which tooth is actually passing the sensor I'd expect you want to control something based on this information.

I think that Johns frequency measurement is a good starting-point. But I also think that you have to extend exactly this PASM driver to do all the number juggling.
At 10000RPM we have a tooth frequency of 10000/60*36 = 6000 teeth/second, right?. I think that it will be hard to do all the calculations you do in calculateCrankRpm can't in SPIN in 1/6000s. Maybe you could start several COGs sharing this task, but Johns PASM-driver has a lot of spare time (in PASM you have >3000 instructions which can be executed in this time) and it's the place where the input-numbers come from, so why not process them there?

Are you ready for PASM? ;o)

turbosupra
03-20-2012, 02:28 PM
It may be time that I learn PASM, but I'll be honest with you, I know literally 0 about it and have avoided it up until this point. Spin was challenging for me, because it was strange compared to the OO language I am most comfortable with, c sharp. Your math is correct, and I believe you are saying that >3000 instructions can be executed in 13333 clk cycles? Wow.

I will say that I am ready if you are willing to teach/guide, I'll do my best and hope to not frustrate you while trying to learn. :)

I scoped the sensor a while back, and found the pictures. The first is what the OEM manufacturer says about the signal, the next picture is of the sensor at 1000rpm (600 teeth a second), the third picture is of the sensor at 5000rpm (3000 teeth a second). I was planning on tapping into this signal (the factory computer uses this signal) with a resistor bridge and using it for controlling other things, you are correct about that. I built an application in c sharp that allows me to receive the data and display it in real time, as well as log it to an excel spreadsheet line by line.



http://img9.imageshack.us/img9/8278/cranksensor1.png


http://img534.imageshack.us/img534/3848/cranksensoridlemeasurem.png


http://img713.imageshack.us/img713/6927/cranksensor5000rpmmeasu.png

turbosupra
03-20-2012, 05:19 PM
Ok, so it appears that the following portion of the jm_freqin object has the equivalent of the variables/constants section at the bottom, with the constants in all caps and the variable names in lowercase?


' --------------------------------------------------------------------------------------------------

POS_DETECT long %01000 << 26
NEG_DETECT long %01100 << 26

tmp1 res 1
tmp2 res 1

mask res 1 ' mask for frequency input pin
cyclepntr res 1 ' hub address of cycle count
cycles res 1 ' cycles in input period

fit 492


Then method frcntr is started with its own cog, I don't understand why the pin is passed as a stack address long though. Or is cognew different in PASM because it automatically has a 492 long stack, so it doesn't need the stack defined?

Then the pin number is copied to tmp1, and after that the program will read the value of tmp1 and store it in tmp2.

After that, the time between positive edges is assigned to ctra register, then the value of tmp2 is added to ctra, not sure how this works but it has to be assigning the pin number to ctra. Then mov adds a literal 1 to frqa, I believe when ctra detects a rising edge.

The same is done for ctrb with falling edges in the next section.

Then the literal value of 1 is copied to mask, followed by mask being shifted left by the number of bits in tmp2 (the pin number), then a bitwise and operation is performed to dira on the ! of mask, possibly alternating back and forth between input and output?

After that a literal 4 is added to tmp1, then tmp1 is copied to cyclepntr

Then it appears to go into a loop.

wait for pin to not equal high (is this comparing the pin to itself and waits for it to equal something set in the register?)
move a literal 0 to phsa to clear it
wait for to equal high (looks like it compares it to itself again and waits for it to equal something set in the register?)
move a literal 0 to phsb

wait for pin to not equal high (looks like it compares it to itself again and waits for it to equal something set in the register?)
move value of phsa to cycles
wait for pin to equal high (looks like it compares it to itself again and waits for it to equal something set in the register?)
add phsb to cycles containing phsa cycles already
write value of cycles to cyclepntr

goto restart


I don't see how this interacts and transfers the data to the spin portion. Does an assembly cognew automatically return a value when you assign it to a variable, like what is done here in the first spin method with okay and cog? Even if that were true, I still can't figure out how the assembly data is passed into the spin code? My brain hurts now, but I've done my best to try and understand the code.




dat

org 0

frcntr mov tmp1, par ' start of structure
rdlong tmp2, tmp1 ' get pin#

mov ctra, POS_DETECT ' ctra measures high phase
add ctra, tmp2
mov frqa, #1

mov ctrb, NEG_DETECT ' ctrb measures low phase
add ctrb, tmp2
mov frqb, #1

mov mask, #1 ' create pin mask
shl mask, tmp2
andn dira, mask ' input in this cog

add tmp1, #4
mov cyclepntr, tmp1 ' save address of hub storage

restart waitpne mask, mask ' wait for 0 phase
mov phsa, #0 ' clear high phase counter

highphase waitpeq mask, mask ' wait for pin == 1
mov phsb, #0 ' clear low phase counter

lowphase waitpne mask, mask ' wait for pin == 0
mov cycles, phsa ' capture high phase cycles

endcycle waitpeq mask, mask ' let low phase finish
add cycles, phsb ' add low phase cycles
wrlong cycles, cyclepntr ' update hub

jmp #restart

MagIO2
03-20-2012, 06:17 PM
POS_DETECT long %01000<<26

is equal to having data in a DAT-section.

tmp1 res 1

is equal to having a local variable which needs to be initialized before first usage. It's very important to have res definitions at the end of one PASM section with no code or data following, because it will confuse the address-counter.

The cognew for a PASM code has a different semantic. For a SPIN-cog it's pre-defined that the 2nd parameter has to point to a stack-space that can be used by the SPIN interpreter. For a PASM - COG you are the master of the second parameter. You can use it to pass values (except the 2 least significant bits which are always %00) or you pass a pointer into HUB RAM that is used by the remaining SPIN-COG and the new PASM-COG for communication.

It's not the pin-number that is stored in tmp1, it's the address which points to the variable fcPin. With the readlong it actually copys the content of fcPin (HUB-RAM) into tmp2 (COG-RAM).

The ANDN is deleting all bits which are set in mask - in this case only the bit representing the input. Actually this step is not needed, because it's guaranteed that all PINs are input for the COG when starting it.

Adding 4 simply forwards the pointer tmp1 which pointed to fcPin to the next variable in HUB-RAM (fcCycles). That's where the COg wants to store the result of the measurement (the length of a whole cycled in clockticks).
The wrlong is copying the value from COG-RAM to HUB-RAM and the SPIN-portion can directly read it from there using fcCycles anywhere it wants.

turbosupra
03-20-2012, 06:54 PM
POS_DETECT long %01000<<26

is equal to having data in a DAT-section.

tmp1 res 1

is equal to having a local variable which needs to be initialized before first usage. It's very important to have res definitions at the end of one PASM section with no code or data following, because it will confuse the address-counter.

The cognew for a PASM code has a different semantic. For a SPIN-cog it's pre-defined that the 2nd parameter has to point to a stack-space that can be used by the SPIN interpreter. For a PASM - COG you are the master of the second parameter. You can use it to pass values (except the 2 least significant bits which are always %00) or you pass a pointer into HUB RAM that is used by the remaining SPIN-COG and the new PASM-COG for communication.

Is this why there is a shl command, to strip those 2 LSB's?



It's not the pin-number that is stored in tmp1, it's the address which points to the variable fcPin. With the readlong it actually copys the content of fcPin (HUB-RAM) into tmp2 (COG-RAM).

The ANDN is deleting all bits which are set in mask - in this case only the bit representing the input. Actually this step is not needed, because it's guaranteed that all PINs are input for the COG when starting it.

Adding 4 simply forwards the pointer tmp1 which pointed to fcPin to the next variable in HUB-RAM (fcCycles). That's where the COg wants to store the result of the measurement (the length of a whole cycled in clockticks).
The wrlong is copying the value from COG-RAM to HUB-RAM and the SPIN-portion can directly read it from there using fcCycles anywhere it wants.

I must be missing something obvious, because I still don't see how cyclepntr interacts with fcCycles anywhere in the code? What is the point of the mask? And what do you suggest I modify this object to do? Should I have all of the math done in the PASM section so that I can query the object for a value and get the return from my main code, or?

MagIO2
03-20-2012, 09:20 PM
No, the shl is creating the bitmask for dira and waitpne/waitpeq

let's say pinNr is 5
%00000000_00000000_00000000_00000001 << pinNr
%00000000_00000000_00000000_00100000

The LSB being cleared means that you can only pass long alligned addresses. So, if you want to pass the address of a byte you have to be carefull and assure that this address is long alligned.

fcCycle is a variable in HUB-RAM. You can access it's content in SPIN by simply using the variable name. You already know the @ operator?! which gives you the address of a variable instead of it's content. If you know an address you can use an instruction like
long[ @fcCycle ] := 10
to set the value of the variable.
cyclepntr := @fcCycle
long[ cycleptr ] := 10
is doing the same. And that's in fact how the PASM code works! cyclepntr is a COG RAM variable which contains the address of the variable fcCycle. And the instruction
wrlong cycle, cyclepntr
is the equivalent of long[cycleptr] := cycle.

I already did some changes to the code. First of all a problem was that I read a lot of zeros in the output. Problem was that jm_frequin deleted the fcCycle after reading! So I removed this part.
jm_frequin was programmed in a way that it only measured each 2nd cycle. I changed it to measure each cycle and for example update another variable which counts theeth.

If you want you can add some code which directly calculates the frequency and stores it in another variable.
You can also add the code for calculating the moving average and store it in another variable.

Here is my current code:

MagIO2
03-20-2012, 09:28 PM
By the way ... thank you for the screenshots ... very interesting! Do you already know how to read the signal with a propeller?

I'd try it with a diode which only let's the positive signal through and a resistor limiting the 5V. But maybe someone more experienced can jump in here and give advice on this.

It would be interesting to see more details - measuring the signal with 1ms per division resolution instead of 20ms per division.

turbosupra
03-21-2012, 03:08 AM
So is this what prop bitmasking is?

http://en.wikipedia.org/wiki/Mask_%28computing%29

It sets 0's to individual bits?

I still don't see how fcCycles in spin is related to Cycles in pasm. I don't see any direct correlation in the code? Is "fc" an alias that points to a pasm variable of the same name?

I am actually the one that set fcCycle to 0, I did this so I knew that I was getting one of 2 things when I called the .freq object, a new value or a 0 and in my code I would just filter out the 0's.

I tried to add the code for calculating the moving average inside of the dat and didn't even get past the first step :) . I tried to add "wrlong (80000000/cycles), rpmptr " to the dat code of stepbystep but I could not get it to do any division? A search on division pasm did not yield any usable results either. Maybe some help with psuedo code would get me going? :)




No, the shl is creating the bitmask for dira and waitpne/waitpeq

let's say pinNr is 5
%00000000_00000000_00000000_00000001 << pinNr
%00000000_00000000_00000000_00100000

The LSB being cleared means that you can only pass long alligned addresses. So, if you want to pass the address of a byte you have to be carefull and assure that this address is long alligned.

fcCycle is a variable in HUB-RAM. You can access it's content in SPIN by simply using the variable name. You already know the @ operator?! which gives you the address of a variable instead of it's content. If you know an address you can use an instruction like
long[ @fcCycle ] := 10
to set the value of the variable.
cyclepntr := @fcCycle
long[ cycleptr ] := 10
is doing the same. And that's in fact how the PASM code works! cyclepntr is a COG RAM variable which contains the address of the variable fcCycle. And the instruction
wrlong cycle, cyclepntr
is the equivalent of long[cycleptr] := cycle.

I already did some changes to the code. First of all a problem was that I read a lot of zeros in the output. Problem was that jm_frequin deleted the fcCycle after reading! So I removed this part.
jm_frequin was programmed in a way that it only measured each 2nd cycle. I changed it to measure each cycle and for example update another variable which counts theeth.

If you want you can add some code which directly calculates the frequency and stores it in another variable.
You can also add the code for calculating the moving average and store it in another variable.

Here is my current code:

turbosupra
03-21-2012, 03:16 AM
I'll try and pull out the scope and get a 1mS reading. I believe I used a 3.9k and 10k resistor ladder, and a forward biased diode to try and read the sensor, but I didn't have the code capable of computing it.

Would you like any other readings while I'm 'under the hood' ?




By the way ... thank you for the screenshots ... very interesting! Do you already know how to read the signal with a propeller?

I'd try it with a diode which only let's the positive signal through and a resistor limiting the 5V. But maybe someone more experienced can jump in here and give advice on this.

It would be interesting to see more details - measuring the signal with 1ms per division resolution instead of 20ms per division.

MagIO2
03-21-2012, 08:56 AM
Bitmask: correct, it's for masking out all the bits not needed or that should not be touched during an operation or for clearing/setting particular bits. So you can use a bitmask for single or multiple bits.
For example: You attached an 8 bit ADC to PINs 8-15 on the propeller. Using INA in PASM will always give you the status of all 32 I/Os. So you'd use a mask $0000_FF00 to make all bits 0 which you are not interested in:
mov reading, ina
and reading, adc_mask
shr reading, #8
With this sequence you have a value in the range from 0 to 255 as you'd expect it from an 8 bit ADC.



SPIN - part:
============
long fcPin ' here it's important to keep the order because only one pointer can
' be passed to PASM
long fcCycles
long fcToothcount
...
okay := cog := cognew(@frcntr, @fcPin) + 1 ' starting COG with frcntr and passing the address of the
' variable fcPin


PASM - part:
============
frcntr mov tmp1, par ' here the PASM stores the address of fcPin in tmp1
' ( this is like tmp1 := @fcPin in SPIN)
rdlong tmp2, tmp1 ' here the content (the pin number) of the variable
' is read and stored in tmp2 ( like tmp2 := fcPin )
...

add cyclepntr, tmp1 ' here 4 is added to the address of fcPin which is
' where you find fcCycles, as fcCycles comes
' directly after fcPin in the SPIN variable definition.
' So indirectly you have cyclepntr := @fcCycles
...

wrlong cycles, cyclepntr ' this will write the content of cycles to the memory
' that cyclepntr points to. So, here you have
' long[ cyclepntr ] := cycles
...

cyclepntr long 4 ' this is predefined with 4 because this is the
' address-offset needed to calculate address of the
' next variable
tmp1 res 1


I'd opt for using a moving average that averages 4,8 or 16 values (2^y), because then you get the division for free. You can simply use a shift operation. If you insist on a division by any other value you have to find some code which divides by any divisor.

turbosupra
03-21-2012, 03:09 PM
Ok, the light bulb is going off above my head, I think I have this.

With pasm's cognew, you can pass the address to the pasm method and one address of a variable which acts as a starting point on a virtual map. To interact with spin and access other parts of the virtual map, you skew the address by X amount of longs and this completes the indirect assignment? If I have this correct, I would have NEVER figured that out on my own so thank you! And for confirming my bit mask understanding.

This pasm division thing is going to take me all day, and that's if I'm lucky, so I'll get to work on that now :)

turbosupra
03-21-2012, 07:18 PM
Ok, at least I can probably provide some comedy with this code. We'll call this ts_brokein.spin :)

I cannot get my pub rpm function to work, I thought that might be a good starting point, and I'm not sure although I think it is the part where I try to define where rpmpntr is in memory.

I'm also not sure you can even do math in pasm the way my commented out portion is trying to do it.






{{

This object uses ctra and ctrb of its own cog to measure the period of an intput waveform.
The period is measured in clock ticks; this value can be divided into the Propeller clock
frequecy to determine the frequency of the input waveform. In application the period is
divided into 10x the clock frequency to increase the resolution to 0.1Hz; this is espeically
helpful for low frequencies. Estimated range is 0.5Hz to ~40MHz (using 80MHz clkfreq).

The counters are setup such that ctra measures the high phase of the input and ctrb measures
low phase. Measuring each phase independently allows the input waveform to be asymmetric.

In order to prevent a loss of signal from causing an eroneous value from the .freq() method
the fcCyles value is cleared after a valid frequency is calculated; this means that you
should not call this method at a rate faster than the expected input frequency.

}}


var

long cog

long fcPin ' frequency counter pin
long fcCycles ' frequency counter cycles
long rpmCycles


pub init(p) : okay

'' Start frequency counter on pin p
'' -- valid input pins are 0..27


if p < 28 ' protect rx, tx, i2c
fcPin := p
fcCycles := 0
okay := cog := cognew(@frcntr, @fcPin) + 1
else
okay := false


pub cleanup

'' Stop frequency counter cog if running

if cog
cogstop(cog~ - 1)


pub period

'' Returns period of input waveform

'return fcCycles
result := fcCycles
fcCycles~


pub freq | p, f

'' Converts input period to frequency
'' -- returns frequency in 0.1Hz units (1Hz = 10 units)
'' -- should not be called faster than expected minimum input frequency

p := period
if p
f := clkfreq * 10 / p ' calculate frequency
fcCycles := 0 ' clear for loss of input
else
f := 0

return f

pub rpm

'return rpmCycles
result := rpmCycles


dat

org 0

frcntr mov tmp1, par ' start of structure
rdlong tmp2, tmp1 ' get pin address location
rdlong tmp3, par ' get beginning variable address location

mov ctra, POS_DETECT ' ctra measures high phase
add ctra, tmp2
mov frqa, #1

mov ctrb, NEG_DETECT ' ctrb measures low phase
add ctrb, tmp2 ' adds val of temp 2, which is the address of par to ctrb
mov frqb, #1

mov mask, #1 ' create pin mask
shl mask, tmp2
andn dira, mask ' input in this cog

add tmp1, #4
mov cyclepntr, tmp1 ' save address of hub storage for spin variable fcCycles

add tmp3, #8
mov rpmpntr, tmp3 ' save address of hub storage for spin variable rpmCycles

restart waitpne mask, mask ' wait for 0 phase
mov phsa, #0 ' clear high phase counter

highphase waitpeq mask, mask ' wait for pin == 1
mov phsb, #0 ' clear low phase counter

lowphase waitpne mask, mask ' wait for pin == 0
mov cycles, phsa ' capture high phase cycles

endcycle waitpeq mask, mask ' let low phase finish
add cycles, phsb ' add low phase cycles
wrlong cycles, cyclepntr ' update hub

{mov previousCycles7, previousCycles6
mov previousCycles6, previousCycles5
mov previousCycles5, previousCycles4
mov previousCycles4, previousCycles3
mov previousCycles3, previousCycles2
mov previousCycles2, previousCycles1
mov previousCycles1, cyclepntr
mov cycleAverage, cyclepntr} '((previousCycles7+previousCycles6+previousCycles5 +previousCycles4+previousCycles3+previousCycles2+c ycles)/8)
wrlong cycles, rpmpntr

jmp #restart

' --------------------------------------------------------------------------------------------------

POS_DETECT long %01000 << 26
NEG_DETECT long %01100 << 26

tmp1 res 1
tmp2 res 1
tmp3 res 1

mask res 1 ' mask for frequency input pin
cyclepntr res 1 ' hub address of cycle count
cycles res 1 ' cycles in input period

rpmpntr res 1
cycleAverage res 1
previousCycles7 res 1
previousCycles6 res 1
previousCycles5 res 1
previousCycles4 res 1
previousCycles3 res 1
previousCycles2 res 1
previousCycles1 res 1

fit 492

MagIO2
03-21-2012, 09:42 PM
N'evening, Turbo ... nice Avatar you have!

Here are my latest changes to the code, enjoy - actually I did enjoy writing it. But maybe you want to do more by yourself? But first your code:
You nearly got adding another variable right! The only problem is in the very beginning:


frcntr mov tmp1, par ' start of structure
rdlong tmp2, tmp1 ' get pin address location
rdlong tmp3, par ' get beginning variable address location

What you really want to do with tmp3 is the same as you do with tmp1 which is a MOV and not a RDLONG. So, if you change it to
mov tmp3, par
it would work.

BUUUUUT ... there is no reason for adding a tmp3 at all! You can use tmp1 again:


add tmp1, #4
mov rpmpntr, tmp1

you have to become aware of those little differences, as the COG-RAM is limited to 496 instructions/variables. That's not too much!

You can also have a closer look to my code. I do initialize all the variables which later hold the pointers with the offset. Then I only have to add - not add + move. This does not save memory because I had to switch the pointer variables from res to long, but it saves runtime - and I like it ;o)
Next step of optimization could be to write all variables back in a bulk-transfer. Then we don't have 10 pointers to 10 different variables but a simple loop which would copy all variables from COG-RAM to HUB-RAM. Maybe as an exercise?

The code you commented out would work, but it's only moving stuff and not calculating something.

The averaging I did is using the method mentioned by Beau in the thread I linked some posts earlier.

turbosupra
03-21-2012, 10:05 PM
Good evening sir! I thought it was about time to reward myself with an avatar, I mean I am learning PASM now :D

Ok, before I look at your code (thank you kindly for the code), I would like to get the following to work as a learning exercise:



pub rpm

'return rpmCycles
result := fcCycles


I've tried every single combo I can think of to just get jm_freq.rpm to return the same values that jm_freq.period returns and I cannot, for the life of me get it to work. I also tried mov tmp3, par somewhere in my hour of attempts, but I have something else wrong because it didn't work. I have a feeling I'm running all around what I want to do, but missing the exact spot.

My biggest uncertainty is still how cog ram interacts with hub ram (I've been doing some reading today, so I believe I'm using the right terminology) . Can you confirm if this is correct?

cognew(@frcntr, @fcPin) allows the second parameter, @fcPin to define a starting address, correct?

add tmp1, #4
mov cyclepntr, tmp1 directs the code to assign the address of par plus 4 bytes (a long) and then assign that address to cyclepntr, correct?

since in the spin var section, fcCycles is the long immediately following the long fcPin or our par address, this is an indirect memory address assignment?



var

long cog

long fcPin ' frequency counter pin
long fcCycles ' frequency counter cycles
long rpmCycles




So I can now do an additional 32 bit shift to indirectly assign rpmpntr to the next long listed in the VAR section by adding another #4 (literal 4 bytes) to the new value of tmp1, which is now currently the address location for fcCycles?



add tmp1, #4 ' tmp1 := address location of tmp1 + a literal 32 bits
mov rpmpntr, tmp3 ' save address of hub storage for spin variable rpmCycles, rpmpntr := tmp3


Sorry this is redundant, I just don't want to build on a misunderstanding and I'm not confident I understood well before, because I feel like I should have been able to get jm_freq.rpm to return the value of fcCycles before if I had understood well, and I could not get it to work.

MagIO2
03-21-2012, 10:36 PM
wrxxxx copies a value from COG RAM to HUB RAM (xxxx being byte, word or long)
rdxxxx copies a value from HUB RAM to COG RAM

Both instructions need a COG RAM address (1st parameter) and a HUB RAM address (2nd parameter). The HUB RAM address is a byte address, as HUB RAM is organized in bytes. But for moving a word the address needs to be alligned to word addresses, for moving longs it needs to be alligned to long addresses. For passing values from one COG to the other both need to know where to read from/write to and that's what the COG that fires up another COG tells it via the 2nd parameter of the COGNEW. It passes the address of the first variable to be used.

For me it looks like you got the point!

And I don't see a reason why the code from post #70 should not work with just that little change (make rdlong a mov).

turbosupra
03-21-2012, 11:04 PM
It works now, I had called the wrong object and was "chasing my tail". Thanks for the confirmation.

I looked at your code, it works awesome and will take me some time to try and digest before I can make an attempt at the bulk transfer loop. I might also try and have it capture the amount of gaps it detects per second.

MagIO2
03-22-2012, 05:40 AM
Oh, by the way ... I think what you really want is to reset the tooth-counter if you detect a gap. That has to be changed as well. Currently it simply goes round and round and whichever tooth it detects first will be number one.
I also think that the counter sould only go up to 33 according to the picture of the wheel.

Sapieha
03-22-2012, 08:05 AM
Hi Mag.

Tooth are for spark and gap are for RPM.



Oh, by the way ... I think what you really want is to reset the tooth-counter if you detect a gap. That has to be changed as well. Currently it simply goes round and round and whichever tooth it detects first will be number one.
I also think that the counter sould only go up to 33 according to the picture of the wheel.

MagIO2
03-22-2012, 08:29 AM
Hi Sapieha!

What I mean is that each tooth on the wheel has a fixed number, right? So, no matter when you start the motor, tooth 1 will be tooth 1 - always and forever. You gave the reason why: A certain tooth will fire up the spark for a certain cylinder, right?
The current code simply starts counting from 1 when you start the COG, no matter which tooth really passes by. The code needs to be changed in a way that it starts counting teeth after seeing the gap passing by once. Then you have a 1 to 1 relation between the tooth passing by with the tooth-number in the COG.

At least this is my current understanding in a matter I have no experience with so far ;o)

This thread somehow reminds me on a german saying:
The blind helps the deaf.
;o)

Me not knowing much about the problem domain and turbo being a newb with propeller.
I enjoy !

Sapieha
03-22-2012, 08:37 AM
Hi Mag.

As You know engine need always know what piston need be fired.
To that You need always start counting from Gap.
And in same time accumulate Gap counts for RPM



Hi Sapieha!

What I mean is that each tooth on the wheel has a fixed number, right? So, no matter when you start the motor, tooth 1 will be tooth 1 - always and forever. You gave the reason why: A certain tooth will fire up the spark for a certain cylinder, right?
The current code simply starts counting from 1 when you start the COG, no matter which tooth really passes by. The code needs to be changed in a way that it starts counting teeth after seeing the gap passing by once. Then you have a 1 to 1 relation between the tooth passing by with the tooth-number in the COG.

At least this is my current understanding in a matter I have no experience with so far ;o)

This thread somehow reminds me on a german saying:
The blind helps the deaf.
;o)

Me not knowing much about the problem domain and turbo being a newb with propeller.
I enjoy !

turbosupra
03-22-2012, 03:05 PM
Hi Mag,

You are correct, and doing pretty well to understand the automotive concepts. Tooth 1 is always tooth one and that is determined by the gap, which is a tooth counting reset. I would have to confirm this, but let's say that on my 4 cylinder engine, cylinder 1 fires at tooth 9, cylinder 2 fires at tooth 18, cylinder 3 fires at tooth 27, cylinder 4 fires at tooth 36 (which tooth 36 technically doesn't exist, so there is probably some sort of offset on my tooth numbers). Both fuel and timing are going to need to be activated for that ignition and fuel injectors appropriate cylinder and the rpm can be counted by determining gaps/bridges per minute. This could probably be counted just from rising edge to rising edge, but the bridge/gap secondary way of doing it allows for a less dynamic measurement. This is a simplistic version of how the ecu uses those sensors to control the engine. I also enjoy this, if only I lived in Hessen or you lived in Maryland, we could start our own car company! A propeller powered car :)

My 6 cylinder actually has the 12 tooth wheel I posted a picture of, which to my surprise has no gap/bridge :o ! I believe it determines which tooth represents which cylinder by also reading the camshaft position sensor, but more testing would be required for me to confirm this.

I'm guessing there is a way to do this, I just have not been able to successfully do the following yet. 10000 rpm is 10000 bridge/gaps per minute or 166.7 bridge/gaps per second. I was trying to count those bridge/gaps per second and display the peak, as a sort of secondary way of measuring rpm.

@Saphieha, thanks for joining!




Hi Sapieha!

What I mean is that each tooth on the wheel has a fixed number, right? So, no matter when you start the motor, tooth 1 will be tooth 1 - always and forever. You gave the reason why: A certain tooth will fire up the spark for a certain cylinder, right?
The current code simply starts counting from 1 when you start the COG, no matter which tooth really passes by. The code needs to be changed in a way that it starts counting teeth after seeing the gap passing by once. Then you have a 1 to 1 relation between the tooth passing by with the tooth-number in the COG.

At least this is my current understanding in a matter I have no experience with so far ;o)

This thread somehow reminds me on a german saying:
The blind helps the deaf.
;o)

Me not knowing much about the problem domain and turbo being a newb with propeller.
I enjoy !

turbosupra
03-22-2012, 05:18 PM
A couple of questions if I could ... what is the difference between



tmp1 res 4
and
tmp1 long 4


When you initialize the variables, you initialize them to hub ram memory address, correct? They do not need to be set to 0 because the hub ram global variable is initialized to 0 by the compiler?

Should I have it try and write in bulk via an array? Or maybe a



wrlong cycles, par
wrlong tooth, par + 4
wrlong average, par + 8


Is that what you meant?










You can also have a closer look to my code. I do initialize all the variables which later hold the pointers with the offset. Then I only have to add - not add + move. This does not save memory because I had to switch the pointer variables from res to long, but it saves runtime - and I like it ;o)
Next step of optimization could be to write all variables back in a bulk-transfer. Then we don't have 10 pointers to 10 different variables but a simple loop which would copy all variables from COG-RAM to HUB-RAM. Maybe as an exercise?

MagIO2
03-22-2012, 09:42 PM
First of all ... ehm ... my guess would be that your 4 cylinder fires at 1, 10, 19, 28. To fire somewhere inside of the gap would be stupid, as you'd have to "create" those teeth via software.

The RES is only reserving COG-RAM memory addresses. These memory locations are not initialized with usefull values. The advantage is that these also don't need HUB-RAM.
And the number behind RES is not a value, it's the number of long - addresses you want to reserve.

With temp1 RES 4 you could for example say something like:
mov temp1, #0
mov temp1+1, #10
mov temp1+2, par
rdlong trmp1+3, temp1+2

temp1 long 4
needs HUB-RAM, as the compiler really stores a 4 at the respective memory location. After loading the PASM code into COG-RAM you will have this position initialized as well.

No, that's not what I meant with bulk write. A bulk write means that all variables where you want to write to (HUB-RAM) are placed one after the other. The variables in COG-RAM are also placed one after the other. In this case you don't need xxxxptr variables for all. And you can do the write in a loop which only needs one wrxxxxx instruction instead of one wrxxxxx per variable. So, from a certain amount of variables is saves COG-RAM.

By the way
wrlong tmp1, par + 4
will compile, but it won't do what you expect!

MagIO2
03-22-2012, 09:52 PM
One important thing is to find out what the pulse / pause ratio will be when reading the real signal. It will tell us how much instructions we can put into the pulse and how much we can put into the pause-part of the code. The current crank simulation creates a 50/50 signal.

Oh ... and it would be interesting which values you need for display only, for further processing and which deviations are allowed!

turbosupra
03-22-2012, 10:09 PM
Be proud Mag! ... Wow did flags hurt my brain, assembly is hard!

This was very hard for me, and I'm sure I couldn't have done it outside of the constructs of your code, but I made a slight addition. I'm also unsure as to how to calculate time in assembly, for example to say

if (current time) => (last time measurement + clkfreq), copy value to valuePeak and reset value to 0, then continue

I started to, but wasn't sure so I commented that part out (starting at line 176)

What do you think? (had to attach externally because of the flash based/active x attachment system on this forum)

turbosupra
03-22-2012, 10:32 PM
Yes, you are probably closer with your tooth numbers, I would have to check TDC on each cylinder and the timing offset at the particular RPM and I would be able to calculate it. It does however use varying degrees of offset, because I've seen timing range from 10 degrees BTDC (before top dead center) to 40 BTDC, all of which would not have a corresponding rising edge.

So res uses cog ram, and long uses hub ram, that makes perfect sense and would allow you to optimize either way based on the code written.

I will try and figure out the bulk loop this evening, let me know what you think of my code that I posted.




First of all ... ehm ... my guess would be that your 4 cylinder fires at 1, 10, 19, 28. To fire somewhere inside of the gap would be stupid, as you'd have to "create" those teeth via software.

The RES is only reserving COG-RAM memory addresses. These memory locations are not initialized with usefull values. The advantage is that these also don't need HUB-RAM.
And the number behind RES is not a value, it's the number of long - addresses you want to reserve.

With temp1 RES 4 you could for example say something like:
mov temp1, #0
mov temp1+1, #10
mov temp1+2, par
rdlong trmp1+3, temp1+2

temp1 long 4
needs HUB-RAM, as the compiler really stores a 4 at the respective memory location. After loading the PASM code into COG-RAM you will have this position initialized as well.

No, that's not what I meant with bulk write. A bulk write means that all variables where you want to write to (HUB-RAM) are placed one after the other. The variables in COG-RAM are also placed one after the other. In this case you don't need xxxxptr variables for all. And you can do the write in a loop which only needs one wrxxxxx instruction instead of one wrxxxxx per variable. So, from a certain amount of variables is saves COG-RAM.

By the way
wrlong tmp1, par + 4
will compile, but it won't do what you expect!


One important thing is to find out what the pulse / pause ratio will be when reading the real signal. It will tell us how much instructions we can put into the pulse and how much we can put into the pause-part of the code. The current crank simulation creates a 50/50 signal.

Oh ... and it would be interesting which values you need for display only, for further processing and which deviations are allowed!

turbosupra
03-23-2012, 06:07 PM
(I will get a scope of the tooth pulse width on and off this weekend)

Ok, I'm not sure how to do the bulk write logically, with the wrlong's being tied to IF statements. I tried the following, but it does not work. I believe you were implying that I should do something along the lines of:

wrLongLoop
wrlong localvar, pntr to par
add localvar, 1
ad pntr, 1
jmp #wrLongLoop

I'm not sure how to do a repeat # of times, so I guessed with the loopCount, else it'll be endless.





mov loopCount, #10
wrlongLoop


wrlong cycles, cyclepntr
add cycles, 1
add cyclepntr, tmp1
sub loopCount, #1
cmp loopCount, #1 WZ,WC 'C = 1 if value1 < value2 'Z = 1 if value1 = value2
if_NC_OR_Z jmp #wrlongLoop ' if value 1 is not less than or equal to #1



On a positive note, I can now calculate gap/bridge based rpm :D :D ... thank you for prodding me towards PASM, it is bad@ss!


http://img441.imageshack.us/img441/9559/gaprpmcalculated1.jpg

MagIO2
03-23-2012, 07:06 PM
Have a look at the DJNZ instruction, this will make your loop a bit more effective, as you don't need to decrement the counter and compare the result.

Nice to see you're progressing.

turbosupra
03-23-2012, 08:04 PM
Thank you, although the credit is owed to you. 5 days ago I couldn't read a single PASM line/command, so I much appreciate the guidance and help.

I'm still not sure how you would do the bulk write with values that are only written based on a flag result, with an if_x statement? Can that be done?

Taking the " if_x wrlong " out of the equation, how does the logic below look? It doesn't work, but I think this is because of the wrlongs I can't comment out that are attached to an if_x statement.




mov loopCount, #10
wrlongLoop

wrlong cycles, cyclepntr
add cycles, 1
add cyclepntr, tmp1
djnz loopCount, #wrlongLoop

MagIO2
03-23-2012, 08:22 PM
Ok ... I already mentioned that bothe variables (the COG-RAM variables and the HUB-RAM variables need to be in a row and in the same order). So, assumning cycles is the first COG-RAM variable and cyclepntr points to the first HUB-RAM variable, you have to do it like that:



CON
BULK_NUM_OF_LONGS = 10
...
DAT
...
mov loopCount, #BULK_NUM_OF_LONGS
wrlongLoop
wrlong cycles, cyclepntr
add wrlongLoop, add1toDest
add cyclepntr, #4
djnz loopCount, #wrlongLoop
' cleanup the changes, so that everything works as before when coming back
mov wrlongLoop, resetWrlong
sub cyclepntr, #(BULK_NUM_OF_LONGS<<2)
....

resetWrlong wrlong cycles, cyclepntr
add1toDest long %1_000000000

The point is, that you do not want to increment the content of cycles, what add cycles, #1 would do. You want that the wrlong instruction uses cycles+1 as the address of the next iteration. This you have to do by using self-modifying code. The least significant 9 bits of an instruction are the source address, the bits from 9-17 contain the destination address. This is what needs to be incremented.

Cyclepntr on the other hand contains the address, so it is fine to add here. But as HUB-RAM addresses are byte-addresses, you have to add #4 to forward to the next LONG.

That's fun, isn't it?? ;o)

I did not test this code and I'm not sure if the compiler complains about #(BULK_NUM_OF_LONGS<<2), but I'm sure you'd find a solution if it would complain.

I don't understand why you want to write conditionally? Simply write again won't hurt and checking costs runtime instead of saving runtime.

turbosupra
03-23-2012, 10:39 PM
Yes this fun! I can't believe how fast PASM is and I'm starting to see the connection clearer between cog ram (longs) and hub ram (bytes) .

The reason I use conditions is because of the way I'm having the values updated with the gap/bridge detection, as well as the max amount of teeth it detects, so it only updates when it sees peak values. Maybe this is the wrong way to go about it, but that is how I wrote the code and it is working, if there is a better way to write that, I'll be happy to migrate to that way.

Here is an archive of the code with the bulk write section not done yet and the variables not ordered yet which will better explain about why I used conditions.

MagIO2
03-23-2012, 11:20 PM
Ok, I see ... 2 solutions:
1. Either have 2 ways of writing values to HUB-RAM. One way would be the old way with separate wrlongs, the other way would be a bulk write of the rest. You'd still use separate wrs for those values that are written conditionally and the bulk write for the others. From a certain amount of variables it will save COG-RAM.

2. Or have 2 variables for the conditional write-values. 1 will keep the valid value and will be copied to HUB-RAM periodically. The other one is the dynamic one, which is moved to the "static" variable where you currently have the conditional wr-instructions.
Then the bulk read will be fine and even if you have to duplicate 2 variables save some RAM.

And here another find:


mov tempCnt, cnt ' * copy count to tempCnt
sub tempCnt, timeStamp ' * subtract initial timeStamp from tempCnt
cmp tempCnt, delta WZ,WC 'test if delta time reached or exceeded, C should be 1 or Z should be 1

There is a trick which saves the tempCnt variable:


mov cnt, cnt ' * copy count to tempCnt
sub cnt, timeStamp ' * subtract initial timeStamp from tempCnt
cmp cnt, delta WZ,WC 'test if delta time reached or exceeded, C should be 1 or Z should be 1

This makes use of the fact that each SPR (special function register - like cnt) also has some shadow-RAM behind. This shadow RAM is used if the SPR name is used as destination.

Some fine-tuning of the code is also needed! I think it makes more sense to remove the hardcode of the dontcount-value. It has to be dynamic! Otherwise you'll get no more updates of the average if the RPM is below a certain value. (A normal tooth will exceed the dontcount.)

turbosupra
03-25-2012, 06:00 PM
Good afternoon,

I was able to get the scope out to the car and get some samples today, I can get more with different time divisions if that will help.

The files are named to reflect the approximate rpm and time division, there names are:


36-2(idling)ScopeAt1mS.jpg
36-2(idling)ScopeAt5mS.jpg
36-2(idling)ScopeAt10mS.jpg
36-2(5000rpm)ScopeAt1mS.jpg
36-2(5000rpm)ScopeAt2mS.jpg
36-2(5000rpm)ScopeAt5mS.jpg


idle is approximately 900rpms. I will say that the bridge or gap looks morel like a gap then a bridge based on the scope, would you agree? I have not pulled the motor apart to see yet, and I know that would be the ultimate confirmation, but to me it appears as a gap or low based on the scopes readings. Let me know if you'd like to see more readings.

turbosupra
03-25-2012, 06:07 PM
I also took the opportunity to get some cam sensor readings as well, the cam sensor uses a 3 trigger wheel with teeth offset, I do not have the exact degree spacing, but I believe they are spaced approximately at 100degrees, 100 degrees and 145 degrees with about 5 degrees of width per trigger wheel. This is not exact or measured yet, but it reflects the large "gap" between the 3rd trigger wheel and the camshaft rotating around completing a full revolution before it passes the 1st trigger wheel by the sensor again.

These files I tried to name appropriately also



3TrigCamSensor(idle)ScopeAt20mS.jpg
3TrigCamSensor(5000rpm)ScopeAt5mS.jpg
3TrigCamSensor(idle)ScopeAt50mS.jpg
3TrigCamSensor(idle)ScopeAt100mS.jpg
3TrigCamSensor(5000rpm)ScopeAt10mS.jpg
3TrigCamSensor(5000rpm)ScopeAt20mS.jpg


idle is again approximately 900rpm and I can get different rpm and time division readings if you would like to see those.

When I come home tonight I am going to try and write some code reflecting the things you've said in your last post.

MagIO2
03-25-2012, 10:26 PM
Nice images!

But what I don't understand ... why don't you try to get the most information out of it? If you compare the 5000/1ms crank reading with the 5000/20ms reading, the first one is much better, right? And with a bigger sample rate it would be even better than that. But OK, most interesting so far is that it's a symetrical alternating voltage somewhere close to 5V @ 700RPM and 10V @ 5000RPM.

Just did some calculations and it looks like the idle RPM is more in 700 range.

The prop scope has 2 channels, right? Why didn't you measure the crank and the cam at once? You'd then see how both signals are related. ;o)

Did you already tell what all this efford is good for? What's your goal? Just out of curiosity .. and if you are willing/allowed to share this information.

turbosupra
03-26-2012, 01:48 AM
Hi,

With your question, do you mean why did I not go to the point where a single sine wave encompassed the entire prop scope screen? I did a couple of different samples because I wanted to show the bridge/gap in at least one of them since this signal is different than your average signal, in that it physically disappears after 34 cycles for 2 of the cycles. I can do a few different samples or overlay them with the second channel if you'd like, just let me know the scenarios and I'll grab them. I should have overlayed them, I just didn't think about it :D . 700rpm may be correct, the factory tachometer isn't known for its accuracy and I was looking at it from the passenger seats angle as well.

My ultimate goal is to be able to adjust the cam angle in relation to the crank angle, which the factory ecu currently does right now. It has an oil pressure controlled gear, of which the oil pressure is modulated by a solenoid. It can change the cam angle up to 30 degrees before or after the "0 angle" location that it would be stationary at, if the auto manufacturer had decided to not make it adjustable.

If I can get that far, there are then a few other things I would like to do in addition to that.


I have gotten the bulk write to partially work, and I'm not 100% sure why. Some of the values are working and some are not, maybe you can explain what I didn't do correctly (code is attached)?

turbosupra
03-27-2012, 08:03 PM
Hello,

I'm still struggling with the bulk write, I drew out a chart with cells on how cog ram maps to hub ram, to try and help myself visualize this. By the way, can I tell PASM to write to the hex address itself and not a pointer to an address?

So the longs in hub ram map to par, par + 1, par + 2, par + 3 etc, (assuming it has been coded that way) . If par was @1B, then cog ram would map to $1B - $1E? and par + 1 would map to $1F - $22, par + 2 would map to $23 - $26 etc ... is this correct? Do the numerical values at the bottom, where the variables for PASM are instantiated, represent an allocation number of bytes? Is that why they are ordered as 4, 8, 12, 16, 20, 24, 28, 32? I think this will be easier for me to understand, if I can picture in my head how the values are mapped and how those numerical columns affect things.

If so, I'm not sure why my code is not working properly, basically it is supposed to follow the logic of

cog ram long, write to par
cog ram long + 1, write to par + 4
cog ram long + 2, write to par + 8

Is this logic at least correct?


Here is the variable value definition section:


org 0

frcntr mov tmp1, par ' * start of structure, passes an address to the value par represents and copies that address to tmp1
rdlong tmp2, tmp1 ' * get beginning variable address location, copy into tmp2

mov ctra, POS_DETECT ' * ctra measures high phase
add ctra, tmp2 ' * add tmp2 to ctra and write to ctra
mov frqa, #1 ' * move frqa 1 byte?

mov ctrb, NEG_DETECT ' * ctrb measures low phase
add ctrb, tmp2 ' * add tmp2 to ctrb and write to ctrb
mov frqb, #1 ' * move frqb 1 byte?

mov mask, #1 ' * create pin mask
shl mask, tmp2 ' * shift left the mask, number of bytes of tmp2?

' save addresses of hub storage
add cyclepntr, tmp1 ' * adds value of cyclpntr and tmp1 and writes that to cyclepntr
add toothcntptr, tmp1 ' * adds value of toothcntptr and tmp1 and writes that to cyclepntr
add averageptr, tmp1 ' * adds value of averageptr and tmp1 and writes that to cyclepntr
add maxptr, tmp1 ' * adds value of maxptr and tmp1 and writes that to cyclepntr
add minptr, tmp1 ' * adds value of minptr and tmp1 and writes that to cyclepntr
' add averageptr, tmp1 ' * adds value of averageptr and tmp1 and writes that to cyclepntr
add toothCntMaxptr, tmp1 ' * adds value of minptr and tmp1 and writes that to cyclepntr
add gapCntAsmptr, tmp1
add gapCntAsmPeakptr, tmp1

mov timeStamp, cnt


The method I'm trying to get working:



mov loopCount, #BULK_NUM_OF_LONGS
mov varLongCount, #0
wrlongLoop
wrlong cycles+varLongCount, cyclepntr ' * write cycles to cyclepntr
add wrlongLoop, add1toDest ' * add 1 to the wrlongLoop value for the djnz
add cyclepntr, #4 ' * add 4 bytes since we are referring to a byte addressed hub ram value, goes to value in hub ram defined after cyclepntr
add varLongCount, #1
djnz loopCount, #wrlongLoop
' Cleanup below
mov wrlongLoop, resetWrlong
sub cyclepntr, #(BULK_NUM_OF_LONGS<<2)

resetWrlong wrlong cycles, cyclepntr



The variable name assignment:


' --------------------------------------------------------------------------------------------------

POS_DETECT long %01000 << 26
NEG_DETECT long %01100 << 26

cyclepntr long 4
toothcntptr long 8
averageptr long 12
maxptr long 16
minptr long 20
toothCntMaxPtr long 24
gapCntAsmptr long 28
gapCntAsmPeakptr long 32

maxc long 0
minc long -1

tooth long 0
toothCntMaxAsm long 0
gapCntAsm long 0
gapCntAsmPeak long 0
timeStamp long 0
delta long 80_000_000
tempCnt long 0

loopCount long 0
varLongCount long 0

database long 0
averagex
avgcnt long 8
dontcount long 13400
add1toDest long %1_000000000


tmp1 res 1
tmp2 res 1

mask res 1 ' mask for frequency input pin
cycles res 1 ' cycles in input period


fit 492

MagIO2
03-27-2012, 09:52 PM
Ok ... let's explain par ...
par inside of a COG is like a readonly variable which contains the content of the second parameter of the COGNEW:
cognew( @mypasm, @first_var )
is like
par := @first_var & %11111111_11111111_11111111_11111100

This is why you'll never find a $1b here. Remember, the address has to be long alligned!

par is a pointer to HUB-RAM in our case pointing to the first variable. And even if this address has to be long alligned, it's still a HUB-RAM address which is a byte-address. So, if the first variable is a long you get to the address of the next variable by adding 4 (par + 4). The next long after that has address par + 8 ... and so on.
If you'd then have a number of words in the variable list, you'd only add 2 from one to the next word.

4, 8, 12, 16, 20, 24, 28, 32 are simply the offsets related to tmp1, which has the same value as par. The initialization at the end plus all the "add x, tmp1" instructions work like this:
cyclepntr := par + cyclepntr which is par + 4
toothcntptr := par + toothcntptr -> par + 8
...

The logic of the loop is correct!

The problems you have are:
1. wrlong cycles+varLongCount, cyclepntr ' * write cycles to cyclepntr
does not work as you expect it to work AND
2. you did not understand how to go forward to the next COG-RAM location in the wrlong

About 1.:
cycle is a label which is converted to a COG-RAM address by the compiler. varLongCount is a label which is converted to a COG-RAM address as well. cycle + varLongCount simply adds those addresses. What you expected was use the cycle address an add the content of varLongCount.

About 2.:
In PASM you have to use self-modifying code to forward addresses used by an instruction. If you have a look at an instruction on bit-level things get clearer:
iiiiii_czri_cccc_ddddddddd_sssssssss
i is the instruction opcode iteslf, so you find different values here for mov, sub, add, wrlong .....
czri are the flags that tell the COG whether the carry flag, the zero flag shall be written (WC, WZ), whether the instruction shall be readonly (WR, NR) and whether the source bits are direct addresses ( # ).
cccc are the conditional execution flags ( IF_Z, IF_NZ, IF_B ..... )
ddddddddd is the destination address - for example cycles -address would be placed here.
sssssssss is the source address

So, if you want to increment the destination-address you actually have to add
000000_0000_0000_000000001_000000000
to the instruction itself. That's why I gave you the code
add wrlongLoop, add1toDest

It should have been fully functional - no need to add the varLongCount stuff ;o) The only thing missing is a jump back to a meaningfull loop after the cleanup-code and before the definition of resetWrlong.
The cleanup restores the original instruction of wrlongLoop, so that dest points to cycles again and resets the cyclepntr.

Hope that helped!

MagIO2
03-27-2012, 09:56 PM
And cycles is not in a row with all the other variables you want to write back. According to the current loop it should be the first of all the long variables in COG-RAM and not a res at the end!

kuroneko
03-28-2012, 01:11 AM
par inside of a COG is like a readonly variable which contains the content of the second parameter of the COGNEW:
cognew( @mypasm, @first_var )
is like
par := @first_var & %00000000_00000000_11111111_11111100
FTFY

turbosupra
03-28-2012, 02:14 AM
Hey, thanks for the reply.

I think I have noticed something I didn't realize before, cogram can technically only store 9 bits of data if it were a single long? And the remaining 23 bits in the register are used for other various functions? If this is correct, why is this?

I understand the value of having the hubram values in the proper order, and the corresponding cogram values also being in order but I'm still not able to make this code work completely. I'm not sure what I'm doing wrong but I'm going to try and test with it in the car rather than spend more time trying to figure out why I can't get the bulk write loop to work. Based on what you've told me, my loop is correct now and my order of longs is correct now, so it is something else that is hosing delta t, peak cnt and min. They all work when I have the separate writes though.

I just noticed you can compile and look at the memory addresses in the prop IDE, strangely enough averagex does not have a hex address? Maybe this is my problem? I also thought it was strange that min was set to -1?

MagIO2
03-28-2012, 07:23 AM
" If this is correct, why is this?"
Because you want to have all informations needed to execute an instruction to fit into 32 bit. So, if you want to use direct data the only place to store it is the source address and setting the I-flag (immediate)accordingly. If you'd allow immediates with any size lager than 9 bit you'd have to occupy the next long anyway, so in terms of COG-RAM usage there is really no difference in using registered data vs. having longer immediate data. The difference is in the details of the processor internal circuitry needed. If you want to support different size immediate you need flexible instruction lenght which needs more complex logic. In fact this is what CISC processors really have - a lot of addressing modes and flexible instruction sizes...

"...rather than spend more time trying to figure out why I can't get the bulk write loop to work."
Fine, as I said bulk write is an optimization in terms of COG-RAM usage and not really needed right now as there is still some COG-RAM left for adding more code. I'll take care of it the next days.

"... averagex does not have a hex address?"
averagex and avgcnt use the same address - one memory location, different labels, where each label matches the usage of the memory location. If you have a closer look at the usage of the code, you'll see that in the beginning avgcnt is used and once we have more than 7 samples, the averagex is used. I simply implemented the moving average in a way that it only spits out an average if you have the right number of samples (=8 in this case) in the database. Otherwise you'd need a counter storing the actual number of samples and even more important, you'd really need a division for calculating the right average for the given number of samples. Fix it to 8 you don't need a counter and you can use >>3 to divide the database by 8.

Additional explaination:
mov calcavg, nopinstr ' * move calcavg to nopinstr
This actually replaces the "djnz avgcnt..." with a NOP - an operation which is doing nothing. From there on the averagex-code becomes effective for all following loops and avgcnt is no longer used.

"I also thought it was strange that min was set to -1?"
1 = $0000_0001
-1 = - ($0000_0001) = $FFFF_FFFE + 1 = $FFFF_FFFF which is the biggest number you can store in a long when using unsigned compare
( $FFFF_FFFE + 1) is the two's complement of 1 -> http://en.wikipedia.org/wiki/Two%27s_complement

@kuroneko:
Thanks for that ... nice to have you as a backup!

turbosupra
03-28-2012, 04:09 PM
Woah, another epiphany! Thank you, that was a beautiful explanation and really helps me in picturing it in my head. I feel like Neo and that I have just seen the Matrix :D

So each cog has 512 long registers (I know a few of them are special purpose)
So each cog can work with a maximum of ~500 individual 9 bit values (possibly x 2 with some creative coding) ... if that is true, now I see why you are teaching me early on about register conservation :)
And somewhere around 9000 bits are available for data source and destination usage within ~500 registers per cog.

I've heard risc/cisc but I did not have a general understanding of the terms until now after your comments prompted me to find this. http://www-cs-faculty.stanford.edu/~eroberts/courses/soco/projects/2000-01/risc/risccisc/

I learned 2's compliment a few years ago, I'm going to have to refresh my memory of that with the wiki link.

Honestly, I think being explained the architecture of the chip really helps you understand PASM and why it does things the way that it does, this has been very beneficial.

MagIO2
03-28-2012, 07:57 PM
To be precise, you have 496 longs per COG the rest are special purpose registers. That's why PASM code usually has the

FIT 496

statement at the end. This tells the compiler that the code between ORG 0 and FIT should not extend 496 longs. Otherwise the compiler complains.

Each COG has 496 32bit registers for free usage. It does not care whether you put instructions or data there. If you use instructions you have 9 bits for a destination address and 9 bits for a source address xor a 9 bit immediate value. It's also 9 bit by purpose: the max. value a 9 bit number can store is ???? 511! Starting at 0 this makes 512 different values and make sure that each instruction can address each other register in COG RAM.

If you have stored some data in COG RAM it's your responsibility what the content means. If you want you can pack 3 x 9bit-values in one long. Or 16 x 2 bit values. It's your code which then has to deal with this kind of data.

So, if you want to count how many 9 bit values fit into a COG you have to take this into account - even if I don't really understand what this counting is good for ;o)

turbosupra
04-04-2012, 02:20 AM
Hello Mag ... I finally built a small circuit and went out to test the code. : D

The prop was reading but the calculations were off and I'm trying to figure what I did wrong in my bench preparation/testing? I really thought my bench test scenarios were solid. The following screen capture is what the condition signal looks like on the prop scope and what the pst was reading in relation to that signal at that moment. I'm working on getting the voltage peaks a little closer to 3.3v, but I'm not sure that is affecting my reading of the pulses.

Any thoughts?

http://img192.imageshack.us/img192/7350/cranksignalconditioning.jpg

http://img513.imageshack.us/img513/7350/cranksignalconditioning.jpg

MagIO2
04-04-2012, 08:10 AM
I think I already mentioned it somewhere (post #82 ;o) ... the point is, that
a) Crank.spin creates a symetrical signal, 50% pulse and 50% pause
b) the CrankSensor works asymetrical


restart waitpne mask, mask ' * wait for 0 phase
mov cycles, phsa ' * copies phsA to cycles
mov phsa, #0 ' * clear high phase counter

' here we do nothing but wait for the next high

' wait for pin == 1
highphase waitpeq mask, mask ' * wait for pin to equal 1
add cycles, phsb ' * adds phsB to cycles (already containing phsA)
mov phsb, #0 ' * clear low phase counter

' from here on we do the whole calculations

' write number of cycles measured to fcCycles
wrlong cycles, cyclepntr ' * writes cycles to cyclepntr which is really fcCycles address

' count the teeth
cmp tooth, #36 WZ,WC' *compare unsigned values of tooth and literal number 36, some sort of post fix flags?
if_ae mov tooth, #1 ' *if above or equal, move to tooth
if_b add tooth, #1 ' *if below, add 1 to tooth
.....


So, the code above implies, that the low can be shorter and that the high has to be at least long enough to do all the calculations. And here the worst case scenario counts, so all conditions are in a state that the longest possible branch is taken.

I guess what you currently have in the real setup is, that the low-phase is long and you only have high peaks which are to short.

Can you also record logic-levels with the PropScope? It would be interesting to see those with a good time-resolution, because that's what's used in the end.
Possible solution: Add an inverter to the hardware?

turbosupra
04-04-2012, 04:49 PM
I would agree that they are asymmetrical (I do remember you warning me of that), but looking at the physical teeth, they are not completely out of balance to the point where it would be the tooth was 25% of the size of the gap, if that helps in giving a mental picture.

I did snap another screen shot before going to work this morning, as I remember you had told me previously that you like also seeing the shape of the wave. To me the waves look pretty close, what do you think, do you still feel there isn't enough time during the peaks? I can definitely try and inverter.



With the 3.3v zener

http://img42.imageshack.us/img42/9343/cranksignalconditioned4.jpg



With the 5.1v zener (for comparison)

http://img214.imageshack.us/img214/9343/cranksignalconditioned4.jpg

Sapieha
04-04-2012, 05:02 PM
Hi.

Why not use Smith trigger to condition them from analog to Digital signals.

turbosupra
04-04-2012, 06:10 PM
Hello,

I had considered that in the final design portion of the circuit to give myself some hysteresis, but at this development point I was just hoping to test the code out. I may have to go to a schmitt trigger sooner than I anticipated though. It probably is a good idea, I was trying to start off as simple as possible.



Hi.

Why not use Smith trigger to condition them from analog to Digital signals.

Sapieha
04-04-2012, 06:17 PM
Hi.

My concern are that if You start simple without --- You will work 2 times on same programing
As none of Yours previous timings will be same


Hello,

I had considered that in the final design portion of the circuit to give myself some hysteresis, but at this development point I was just hoping to test the code out. I may have to go to a schmitt trigger sooner than I anticipated though. It probably is a good idea, I was trying to start off as simple as possible.

turbosupra
04-04-2012, 06:59 PM
You make a good point! I don't want to do the programming two times :)

I have a few of these at home somewhere, maybe it is time I used one.

http://www.nxp.com/documents/data_sheet/74HC7014_CNV.pdf

Sapieha
04-04-2012, 07:05 PM
Hi.

Good choice ---- But Ground all unused inputs.




You make a good point! I don't want to do the programming two times :)

I have a few of these at home somewhere, maybe it is time I used one.

http://www.nxp.com/documents/data_sheet/74HC7014_CNV.pdf

turbosupra
04-05-2012, 12:14 AM
What do you think of this signal Mag? It appears "on time" is about 80% of off time, and there is about 2.5-3 peaks during 1mS at 4000rpm, so approximately 1 high every 125 micro seconds at 10000rpm, or about 1 peak every 8000 clk cycles, maybe 40% of that is "on time" so say worst case scenario about 3000 clk cycles at 10000rpm. Do you still feel there is not enough time?



@Sapieha

Here is the comparison on each side of the 466Ω resistor (channel 2 is before, channel 1 is after)

3.3 zener

http://img441.imageshack.us/img441/7043/cranksignalconditioned5.jpg



5.1 zener

http://img31.imageshack.us/img31/7043/cranksignalconditioned5.jpg


And here are two samples on the scope with the schmitt trigger as part of the circuit, one at idle (900rpm) and one at 4000rpm. Channel 2 is the signal going into the schmitt, channel 1 is the signal coming out of the schmitt.

idle

http://img827.imageshack.us/img827/7043/cranksignalconditioned5.jpg



4000rpm

http://img192.imageshack.us/img192/7043/cranksignalconditioned5.jpg

turbosupra
04-05-2012, 04:59 PM
What do you think of the wave shape and can you tell me what you mean by "logic-levels" and then I will record them with the scope?

Below is what the signals look like with the schmitt in place, and they are compatible with the prop. Just to test, I was able to get the old spin code to read the values, but it would choke when I revved the engine up and freeze? The old spin code reads the time from one peak to the next, I'm trying to figure out how the PASM code is different, at idle there is something like 166000 clk cycles in between each logic high, even if 1/10th of that were the time waitpeq had to work, that's still 16k cycles, which I would think would be plenty of time for PASM? To me it looks like the logic low is about 3 times as long as the logic high. I don't know if I can tweak the PASM code on my own, but before I attempt that, do you agree?


http://img684.imageshack.us/img684/7043/cranksignalconditioned5.jpg





So, the code above implies, that the low can be shorter and that the high has to be at least long enough to do all the calculations. And here the worst case scenario counts, so all conditions are in a state that the longest possible branch is taken.

I guess what you currently have in the real setup is, that the low-phase is long and you only have high peaks which are to short.

Can you also record logic-levels with the PropScope? It would be interesting to see those with a good time-resolution, because that's what's used in the end.
Possible solution: Add an inverter to the hardware?


It's actually partially working, the rpm will sometimes be correct, it appears to me it is not recognizing the gaps based on the PST output?

http://img29.imageshack.us/img29/7043/cranksignalconditioned5.jpg
Here is how the long gap looks on the scope at high resolution, it's about 5.5mS at 1000rpm for the long gap
http://img843.imageshack.us/img843/7043/cranksignalconditioned5.jpg


Here is how the standard gaps look at high resolution, it's about 1.5mS at 1000rpm for the standard gaps
http://img849.imageshack.us/img849/7043/cranksignalconditioned5.jpg]

turbosupra
04-06-2012, 10:19 PM
Hi Mag,

I don't know if you're still interested in this, but the hardware circuit I built is giving a square wave signal now (or really close) and I can read it at low rpm's with spin, with my old spin code.

I'd love to be able to read it with PASM so that I don't have to be concerned with speed. Below is the code I used to read the rpm in spin, I'm not sure I can tweak the PASM code on my own, so I'd appreciate your help if you are still interested.



PUB calculateCrankRpm(localCrankPeriod) | elapsed

elapsed := -cnt

l_previousNum8 := l_previousNum7
l_previousNum7 := l_previousNum6
l_previousNum6 := l_previousNum5
l_previousNum5 := l_previousNum4
l_previousNum4 := l_previousNum3
l_previousNum3 := l_previousNum2
l_previousNum2 := l_current
l_current := localCrankPeriod



if (l_timeStampCrank == 0)
l_timeStampCrank := (cnt + clkfreq)

if (l_previousNum8 <> 0 and l_previousNum7 <> 0 and l_previousNum6 <> 0 and l_previousNum5 <> 0 and l_previousNum4 <> 0 and l_previousNum3 <> 0 and l_previousNum2 <> 0)
l_previous7average := ((l_previousNum8 + l_previousNum7 + l_previousNum6 + l_previousNum5 + l_previousNum4 + l_previousNum3 + l_previousNum2)/7)
l_tCnt += 1

if (((l_current/10)*100) > ((l_previous7average/10)* 110)) or (((l_current/10)*100) < ((l_previous7average/10)*90))
l_Cnt += 1

if (cnt > l_timeStampCrank)
l_timeStampCrank := (cnt + clkfreq)
l_CntPeak := l_Cnt ' I should a number here matching see rpm/60, for a frequency of 6000 I should see 166.67 of these each second
l_CntPeakRead := false
l_Cnt := 0

l_current := l_previous7average

else

if (((clkfreq/l_current)*60)/36) < 200
pst.str(string("crap2"))
pst.char(13)

else

l_toothRpm := (((clkfreq/l_previous7average)*60)/36)


elapsed += cnt - 544


Far away
http://img692.imageshack.us/img692/9546/cranksignalconditionedd.jpg

Up close showing the gap and the first tooth (I'm not sure why it elongates the first tooth)
http://img832.imageshack.us/img832/9546/cranksignalconditionedd.jpg

Up close to compare the logic highs and logic lows
http://img585.imageshack.us/img585/9546/cranksignalconditionedd.jpg


And how it looks on the pst (next I'll be cleaning this up so it displays like you set it up)
http://img820.imageshack.us/img820/6870/pstdualtransistors1.jpg


Here is a close up of the crank trigger wheel and the bridge/gap
http://img716.imageshack.us/img716/5378/362cranktrigger1.jpg

MagIO2
04-08-2012, 05:25 PM
Hi Turbo,

I've been off for a while - just watching with my mobile, but it's no fun to write!

Looking at the screenshots I think the problem is that the GAP detection is not working properly. I think I already mentioned somewhere that it needs to be changed?
Anyways, one thing I'd do is add an inverter after your schmitt trigger because on some screenshots it looks like the high-time after the gap is not really longer then the usual tooth-high-time. The low time seems to be more reliable.
Or you can maybe change the code by exchanging both waitpxxx-instructions. Need to think about it. Or you simply try it ;o)

Why exactly do you start with this previousNum thing again? You remember that we already calculate an average using the moving-average code?

turbosupra
04-09-2012, 01:45 AM
Hello,

When I was having trouble reading the frequency with PASM, I wanted to see how it did with spin just as a baseline. The code I posted works at lower rpm, but does not have the speed needed for higher rpm. It was just me verifying that the hardware portion was working.

You are right, gap detection is not working, the rest of the calculations seem to be working though since the new hardware was implemented and I was able to copy and imitate the signal perfectly with the prop. :)

At least I can now test confidently, knowing that the signal I generate with the prop matches the signal the cars hardware is putting out perfectly.

http://img845.imageshack.us/img845/3120/workingpasmpst1.jpg

Here is the code I use to generate the proper signal

crank.spin


CON
' current code only allows from 1Hz upwards
MIN_FREQUENCY = 1
' dunno which upper limit can be achieved by the SPIN-code
MAX_FREQUENCY = 9200

VAR
byte pin, presentTeeth, missingTeeth, cog
long frequencyRate

long pulse, sync, timing, l_localIndex
long stack[100]

PUB start(p, pt, mt, initFrequency)
pin := p
presentTeeth := pt
missingTeeth := mt
setFrequency( initFrequency )

if cog
cogstop( cog-1 )

'cog := cognew( crankPulser, @stack )+1
cog := cognew(simulateCrankWheel(pin, presentTeeth, missingTeeth, initFrequency), @stack )+1 'simulateCrankWheel(4, 36, 2, 15000)

PUB setFrequency( newFrequ )
frequencyRate := (newFrequ #> MIN_FREQUENCY ) <# MAX_FREQUENCY

timing := clkfreq / frequencyRate
pulse := timing >> 1

PUB getFrequency
return frequencyRate

PRI crankPulser
dira[pin] := 1 ' make output

frqa := 1
phsa := 0
ctra := (%00100 << 26) | pin ' setup counter for pin

sync := cnt ' sync with system clock
repeat
repeat l_localIndex from 1 to (presentTeeth- missingTeeth)
phsa := -pulse ' create 1ms pulse
waitcnt(sync += timing) ' hold for current hz setting

repeat l_localIndex from 1 to (missingTeeth)
phsa := 0 ' create 1ms pulse
waitcnt(sync += timing)



PUB simulateCamWheel(simCamWheelPinNum, engineRPM, crankRotsToCamRots, degreeOfTrigger1, degreeOfTrigger2, degreeOfTrigger3, camTriggerDegreeWidth) | localIndex1, crankRpmPerSecond, camRpmPerSecond, localCnt, trigger1Wait, trigger2Wait, trigger3Wait, emtpySpace1Wait, emtpySpace2Wait, emtpySpace3Wait, cyclesPerDegOfRot



dira[simCamWheelPinNum]~~
outa[simCamWheelPinNum]~
localIndex1 := 0

crankRpmPerSecond := (engineRPM/60) ' 166.667 crank rots per second at 10k rpm
camRpmPerSecond := (((engineRPM*10)/60)/2) ' 83.333 cam rots per second at 10k rpm
cyclesPerDegOfRot := (((clkfreq/camRpmPerSecond)/360)*10) ' 960000/360 at 10k or 2660

trigger1Wait := (camTriggerDegreeWidth*cyclesPerDegOfRot) ' 13300 / 5 degrees
trigger2Wait := (camTriggerDegreeWidth*cyclesPerDegOfRot) ' 13300 / 5 degrees
trigger3Wait := (camTriggerDegreeWidth*cyclesPerDegOfRot) ' 13300 / 5 degrees
emtpySpace1Wait := (degreeOfTrigger1*cyclesPerDegOfRot) ' 266000 / 100 degrees
emtpySpace2Wait := (degreeOfTrigger2*cyclesPerDegOfRot) ' 266000 / 100 degrees
emtpySpace3Wait := (degreeOfTrigger3*cyclesPerDegOfRot) ' 385700 / 145 degrees
' + = 957600 (((clkfreq/957600) * 60sec/min) * 2rots campercrank) = 10krpm

localCnt := cnt

repeat

outa[simCamWheelPinNum]~~
waitcnt(localCnt += trigger1Wait) ' 6665
outa[simCamWheelPinNum]~
'waitcnt(localCnt += (clkfreq/521)) ' 160000-6665
waitcnt(localCnt += emtpySpace1Wait)
outa[simCamWheelPinNum]~~
waitcnt(localCnt += trigger2Wait) ' 6665
outa[simCamWheelPinNum]~
'waitcnt(localCnt += (clkfreq/521)) ' 160000-6665
waitcnt(localCnt += emtpySpace2Wait)
outa[simCamWheelPinNum]~~
waitcnt(localCnt += trigger3Wait) ' 6665
outa[simCamWheelPinNum]~
'waitcnt(localCnt += (clkfreq/521)) ' 160000-6665
waitcnt(localCnt += emtpySpace3Wait)


PUB simulateCrankWheel(simCrankWheelPinNum, crankTeethSimToothCnt, crankTeethSimMissingToothCnt, crankTeethSimRPM) | localIndex, waitTime, logicHighWaitTime, logicLowWaitTime, logicHighGapWaitTime, logicLowGapWaitTime, startCnt, existingTeeth



' Initialize variables here
dira[simCrankWheelPinNum]~~
outa[simCrankWheelPinNum]~
localIndex := 0
waitTime := (clkfreq/((crankTeethSimRPM*(crankTeethSimToothCnt*2))/60)) ' for 1500rpm = 44444
logicHighWaitTime := ((waitTime/50)*45)
logicLowWaitTime := ((waitTime/50)*55)
logicHighGapWaitTime := ((((waitTime*6)/100)*43)-logicHighWaitTime) ' this percentage can be measured by a prop scope and a signal capture
logicLowGapWaitTime := ((((waitTime*6)/100)*57)-logicLowWaitTime)
existingTeeth := (crankTeethSimToothCnt - crankTeethSimMissingToothCnt)

startCnt := cnt

{ctra := (%00100 << 26) | simCrankWheelPinNum ' setup counter for pin
frqa := 1 ' this makes the cnt screw up every 26 seconds
phsa := 0}

{
This is for a Hall effect signal/sensor simulation
For 1000rpms on a 36-2 crank trigger wheel, it'd be (1000(36*2)) = 72000, then 72000/60 = 1200 teeth on/teeth off per clkfreq. 80000000(clkfreq)/1200 =
a tooth on or tooth off every 133333.3 clk cycles. It would be on for 66666.67 clk cycles and then was off for every 66666.67 clk cycles
So you'd loop on for 66666.67 cycles and then off for 66666.67 cycles and after 34 times, you'd wait for 66666.67 cycles * 4, to simulate the 2
missing teeth on and accompanying 2 "missing" teeth off (or gaps in between each tooth) . It should account for 2 missing teeth and 3 missing toothgaps
An example to start the method would be simulateCamWheel(7, 3, 105, 100)
}


repeat

repeat localIndex from 1 to (existingTeeth) ' loop number of physical teeth on and off
outa[simCrankWheelPinNum]~~
'l_crankTeethSimOn := startCnt
waitcnt(startCnt += logicHighWaitTime) ' on for 1 tooth
outa[simCrankWheelPinNum]~
'l_crankTeethSimOff := startCnt
waitcnt(startCnt += logicLowWaitTime) ' off for 1 tooth


repeat localIndex from 1 to 1'(crankTeethSimMissingToothCnt) ' loop number of times for each missing tooth
outa[simCrankWheelPinNum]~
'l_crankTeethSimOff := startCnt
waitcnt(startCnt += logicLowGapWaitTime) ' off for 57% of 2 teeth
outa[simCrankWheelPinNum]~~
'l_crankTeethSimOff := startCnt
waitcnt(startCnt += logicHighGapWaitTime) ' on for 43% of 2 teeth






stepbystep.spin



l_rpm := 15000
l_toothCount := 36
l_frequency := ((((l_rpm*10)/60)*l_toothCount)/10)

'co.start(CRANK_WRITE_PIN, 36, 2, l_frequency)
co.start(CRANK_WRITE_PIN, 36, 2, l_rpm)

turbosupra
04-09-2012, 03:22 AM
Hi Mag,

I have some good news, I haven't checked this on the car, only the sim method, but I noticed gap cnt was 2x what it should be, I believe because the a/c gap wave, which is about the width of 4 teeth is being split at the digital conversion so that the digital version (ac wave split in half) has a longer than usual low, but also a longer than usual high, both being about the equivalent width of 2 teeth. Unfortunately it does not work at lower rpm :(

I think this is causing the gap portion to count twice? So when I call the method, I've just done a /2 now.

The question is, how do I go from here?

I don't exactly know how I want to go about this, but I know my end goal. I want to be able to compare the cam sensor location to the crank sensor location and then adjust the cam sensor location via a simple transistor, until the feedback from the cam sensor location matches where I'd like it to be. Maybe I can do a time comparison based on trigger tooth 1 of both the cam sensor trigger and the crank sensor trigger and then calculate the offset? Does that make sense?

The code I use to simulate the cam sensor output, which I wrote this weekend and it simulates the cam sensor output perfectly.

Do you have any guidance on how to reference trigger tooth 1 of each sensor and then offset it the amount of degrees or time accordingly?


http://img24.imageshack.us/img24/4409/workingpasmpst2.jpg



PUB simulateCamWheel(simCamWheelPinNum, engineRPM, crankRotsToCamRots, degreeOfTrigger1, degreeOfTrigger2, degreeOfTrigger3, camTriggerDegreeWidth) | localIndex1, crankRpmPerSecond, camRpmPerSecond, localCnt, trigger1Wait, trigger2Wait, trigger3Wait, emtpySpace1Wait, emtpySpace2Wait, emtpySpace3Wait, cyclesPerDegOfRot



dira[simCamWheelPinNum]~~
outa[simCamWheelPinNum]~
localIndex1 := 0

crankRpmPerSecond := (engineRPM/60) ' 166.667 crank rots per second at 10k rpm
camRpmPerSecond := (((engineRPM*10)/60)/2) ' 83.333 cam rots per second at 10k rpm
cyclesPerDegOfRot := (((clkfreq/camRpmPerSecond)/360)*10) ' 960000/360 at 10k or 2660

trigger1Wait := (camTriggerDegreeWidth*cyclesPerDegOfRot) ' 13300 / 5 degrees
trigger2Wait := (camTriggerDegreeWidth*cyclesPerDegOfRot) ' 13300 / 5 degrees
trigger3Wait := (camTriggerDegreeWidth*cyclesPerDegOfRot) ' 13300 / 5 degrees
emtpySpace1Wait := (degreeOfTrigger1*cyclesPerDegOfRot) ' 266000 / 100 degrees
emtpySpace2Wait := (degreeOfTrigger2*cyclesPerDegOfRot) ' 266000 / 100 degrees
emtpySpace3Wait := (degreeOfTrigger3*cyclesPerDegOfRot) ' 385700 / 145 degrees
' + = 957600 (((clkfreq/957600) * 60sec/min) * 2rots campercrank) = 10krpm

localCnt := cnt

repeat

outa[simCamWheelPinNum]~~
waitcnt(localCnt += trigger1Wait) ' 6665
outa[simCamWheelPinNum]~
'waitcnt(localCnt += (clkfreq/521)) ' 160000-6665
waitcnt(localCnt += emtpySpace1Wait)
outa[simCamWheelPinNum]~~
waitcnt(localCnt += trigger2Wait) ' 6665
outa[simCamWheelPinNum]~
'waitcnt(localCnt += (clkfreq/521)) ' 160000-6665
waitcnt(localCnt += emtpySpace2Wait)
outa[simCamWheelPinNum]~~
waitcnt(localCnt += trigger3Wait) ' 6665
outa[simCamWheelPinNum]~
'waitcnt(localCnt += (clkfreq/521)) ' 160000-6665
waitcnt(localCnt += emtpySpace3Wait)