[RESOLVED] Using buffers and semaphores (lock* functions)

Mightor

I am writing an object to allow me to run the PING))) and its servo on a separate cog. The code keeps track of objects in its vicinity by logging the angle it was at and the distance to the object. This is done with an array of 16 words. When it reaches either the max right or max left angle, it flushes the buffer to a cache, which is also an array of 16 words. The main program can get a copy of the cache using a function called copyCache. I thought I had all the locks in the right place but it seems that when I run the program, I get a discrepancy between the cache and the array that I am copying it to in the main program.

The attached zip files contains everything needed to run it. Please note that right now, the PING))) sensor and servos are not needed. Everything is running in a debugging mode and no pins are modified in any way. This is mainly so I can mess around with this code while at work without needing to carry an entire bot with me. All debugging is done through HyperTerm or some other ANSI compatible Terminal app (I use PuTTY myself).

Here is an example output:

Resetting servo to initial position
swivel
Flushing buffer to cache
Copying cache to map
cache: 69 173 82 148 194 196 63 255 64 26 213 63 186 24 172 239
map: 69 173 82 148 194 196 63 255 64 26 213 63 186 24 172 239
Flushing buffer to cache
Flushing buffer to cache
Flushing buffer to cache
Copying cache to map
cache: [color=#990000]91 148 70 150 247 24 173 34 177 130[/color] 112 68 207 55 209 176
map: [color=#990000]197 247 43 142 52 121 82 17 137 50[/color] 112 68 207 55 209 176
Flushing buffer to cache
Flushing buffer to cache
Flushing buffer to cache
Flushing buffer to cache
Copying cache to map
cache: [color=#990000]57 10[/color] 18 252 39 176 235 163 176 251 61 136 167 130 161 7
map: [color=#990000]27 253[/color] 18 252 39 176 235 163 176 251 61 136 167 130 161 7
Flushing buffer to cache
Flushing buffer to cache
Flushing buffer to cache
Stopping cog number 2

As you can see, the first time it works ok, but the map and cache differ in the 2nd and 3rd copy action. I am really at a loss here. I have read the Propeller Manual and I can't see the problem in my code. Obviously there is one, or I wouldn't be having this issue [noparse]:)[/noparse]

If someone could have a look at it, I'd very grateful.

Thanks,
Mightor

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
| To know recursion, you must first know recursion.

Post Edited (Mightor) : 8/19/2007 2:04:44 PM GMT

deSilva

When you output MAP (lines 40 to 44 in radar_demo) this happens in parallel to _swivel activities. As lines 40 - 44 are not guarded with locks, you see the actual situation, i.e. how far _swivel has accomplished this activities... You see that is NOT very far.

Why are both rows equal in the beginning? I think the COG for _swivel needed some time to be installed..

WRT style:

repeat until not lockset(SemID)
-- this is awkward: write: repeat while lockset(SemID)

repeat while not radar.map_ready
-- ditto

I see you know both variants, but a good practice is to avoid "NOT" as much as possible.

wordfill(@cache, CACHESIZE, 0)
wordmove(@cache, @buffer, CACHESIZE)
-- wordfill is redundant

Post Edited (deSilva) : 8/19/2007 1:32:01 PM GMT

Mightor

deSilva said...
When you output MAP (lines 40 to 44 in radar_demo) this happens in parallel to _swivel activities. As lines 40 - 44 are not guarded with locks, you see the actual situation, i.e. how far _swivel has accomplished this activities... You see that is NOT very far.

Why are both rows equal in the beginning? I think the COG for _swivel needed some time to be installed..

WRT style:

repeat until not lockset(SemID)
-- this is awkward: write: repeat while lockset(SemID)

repeat while not radar.map_ready
-- ditto

wordfill(@cache, CACHESIZE, 0)
wordmove(@cache, @buffer, CACHESIZE)
-- wordfill is redundant

I didn't think lines 40-44 needed to be guarded with a semaphore due to the fact that I am dealing with a copy of the cache. The actual copying of the cache to the map variable *is* guarded with a semaphore. I am not sure what putting a semaphore around those lines would achieve, this data is static until I call copyCache again. All the locking should be dealt with within the Radar module, not the one calling it.

The code to retrieve and print out the values of the map var in radar_demo aren't run until swivel has made at least ONE 180 sweep (everytime the text "Flushing buffer to cache" appears, this signifies a 180 degree swivel has been completed), this ensure that when copyCache is called there will always be at least a single run's worth of data in there. When copyCache is called, it also locks the _swivel function out of writing to its own buffer, at least while it's copying.

Heh, as for style, I copied those lockset lines straight from the Prop manual [noparse]:)[/noparse] (it's good when you have someone to blame shift to :P)

I think I might be missing something from your explanation, it's left me somewhat puzzled.

Gr,
Mightor

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
| To know recursion, you must first know recursion.

Mightor

deSilva,

I have figured out the problem, on line 121 in copyCache, I had the following:

PUB copyCache(mapPtr) | i
  repeat while lockset(SemID)
  Debug.str(string("Copying cache to map", 10, 13))
  Debug.str(string("cache: "))
  repeat i from 0 to CACHESIZE - 1
    Debug.dec(map[noparse][[/noparse] i ])                                      ' this is line 121
    Debug.str(string(" "))
  Debug.str(string(10,13)) 
  wordmove(mapPtr, @cache, CACHESIZE)
  lockclr(semID)

That ought to read cache[noparse][[/noparse] i ], of course! Doh! When I change the code to

PUB copyCache(mapPtr) | i
  repeat while lockset(SemID)
  Debug.str(string("Copying cache to map", 10, 13))
  Debug.str(string("cache: "))
  repeat i from 0 to CACHESIZE - 1
    Debug.dec(cache[noparse][[/noparse] i ])
    Debug.str(string(" "))
  Debug.str(string(10,13)) 
  wordmove(mapPtr, @cache, CACHESIZE)
  lockclr(semID)  

It works flawlessly! Thanks for your time though and as you can see I change the lockset lines to the more stylish repeat while type [noparse]:)[/noparse]

Gr,
Mightor

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
| To know recursion, you must first know recursion.

deSilva

Mightor, I was close after you explained the "Copying" But it is difficult to read in another persons program... Why do you use the locking in the first place? You will need in only for multiple writings, not for just reading !

Mightor

deSilva said...
Mightor, I was close after you explained the "Copying" But it is difficult to read in another persons program... Why do you use the locking in the first place? You will need in only for multiple writings, not for just reading !

I just don't want any writing to happen when the buffers/caches are being copied/read. I want the cache to be a representation of a single sweep, not necessarily the very last data. I might fine tune the locking mechanism at some point, have one for reading/writing the buffer and one for reading and writing the cache. Right now I'm happy that it works [noparse]:)[/noparse]

Gr,
Mightor

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
| To know recursion, you must first know recursion.