Need code double checked, plz

computer guy · 2010-01-07 10:08

Could someone please take the time to look at the following code. I think it is almost right however it isn't working as I expect it to.
BT.spin


CON

StartDelimiter                 = $02     ' STX
EndDelimiter                   = $03     ' ETX
REQ_PACKET                     = $52     ' R

GAP_WRITE_LOCAL_NAME           = $04

VAR

Byte dataSet[noparse][[/noparse] 340], rxBuffer[noparse][[/noparse] 16]

Long Started
OBJ
                                                                                   
serial : "Full_Duplex_Serial"

Pub Start (RXpin, TXPin, Mode, Baud) : okay | ptr

    Started := serial.start(RXpin, TXpin, Mode, Baud)
    if Started == -1
      return Started
    else
      Started := true
      return Started
      
PUB Stop
    serial.stop
    Started := false

PUB calculate_crc(data, size) | crc

  crc := 0
  crc := data[noparse][[/noparse] 1] + data[noparse][[/noparse] 2] + data[noparse][[/noparse] 3] + data[noparse][[/noparse] 4]
  crc := crc & $ff
  return(crc)

PUB receive_response | read_data, num_bytes, i

  num_bytes := 0
  i := 0
  repeat
    num_bytes++
    read_data[noparse][[/noparse] i] := serial.rx
    i++
  while (serial.rxcheck > 0)
  return read_data
    
PUB Set_Local_Name (name) | len, ptr, temp

  len := strsize(name)
  byte[noparse][[/noparse] temp++] := strsize(name)
  Repeat ptr from 0 to strsize(name)
    byte[noparse][[/noparse] temp++] := byte[noparse][[/noparse] name++]
  'byte[noparse][[/noparse] temp++] := $00
  sendREQ (GAP_WRITE_LOCAL_NAME, temp, len)

PUB send_cmd_receive_response(data, size) | crc, i

  crc := calculate_crc(data, size)
  data[noparse][[/noparse] 5] := crc
  repeat i from 0 to size
    serial.tx(data[noparse][[/noparse] i])
  waitcnt(clkfreq * 3 / 10 + cnt)
  return receive_response

PUB sendREQ (opCode, msg, length) | ptr, dataLen

  if Started
    dataSet := $00
    dataLen := length + 1
    ptr := 0
    dataSet[noparse][[/noparse] ptr++] := StartDelimiter
    dataSet[noparse][[/noparse] ptr++] := REQ_PACKET
    dataSet[noparse][[/noparse] ptr++] := opCode
    dataSet[noparse][[/noparse] ptr++] := dataLen
    dataSet[noparse][[/noparse] ptr++] := $00 
    Repeat strsize(msg)                                  ' Repeat for the length of the message
      dataSet[noparse][[/noparse] ptr++] := byte[noparse][[/noparse] msg++]
    dataSet[noparse][[/noparse] ptr++] := EndDelimiter
    send_cmd_receive_response(dataSet, ptr + 1)

Test.spin

OBJ

  BT    : "BT"

PUB Main

  BT.Start(20,21,0, 9600)
  BT.Set_Local_Name(string("EcoSure UI"))

I am expecting it to send the following to pin 21, however it is sending something totally different.

$02, $52, $04, $0C, $00, $61, $0B, $45, $63, $6F, $53, $75, $72, $65, $20, $55, $49, $00, $03

Any Ideas?

Thank you

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
"powered by Propeller" domed stickers $1.50 - Find them here
Check out my Design and Technology project for my Higher School Certificate www.ecosureblog.net

Post Edited (computer guy) : 1/7/2010 10:35:06 AM GMT

kuroneko · 2010-01-07 10:41

PUB Set_Local_Name (name) | len, ptr, temp
  len := strsize(name)
  byte[noparse][[/noparse] temp++] := strsize(name)
  Repeat ptr from 0 to strsize(name)
    byte[noparse][[/noparse] temp++] := byte[noparse][[/noparse] name++]
  'byte[noparse][[/noparse] temp++] := $00
  sendREQ (GAP_WRITE_LOCAL_NAME, temp, len)

Just from a quick glance, where do you think byte[noparse][[/noparse]temp++] is going (temp being local and uninitialised)?

Post Edited (kuroneko) : 1/7/2010 11:01:34 AM GMT

computer guy · 2010-01-07 11:05

I am trying to set the first byte of temp to strsize(name) and then the rest of temp with the string itself.
e.g.

name = "TEST"

so strsize(name) = 4

so temp = $04, $54, $45, $53, $54

I modified some code from a similar object.
Guess I don't understand what it's doing.

Thank you

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
"powered by Propeller" domed stickers $1.50 - Find them here
Check out my Design and Technology project for my Higher School Certificate www.ecosureblog.net

kuroneko · 2010-01-07 11:13

computer guy said...
... so temp = $04, $54, $45, $53, $54 ...

temp is a local variable with a random value (size 4 bytes, i.e. a long). byte[noparse][[/noparse]temp++] addresses hub memory byte-wise starting at address temp. What you need (assumption) is some temporary buffer (VAR byte buffer[noparse][[/noparse]N]) which can at least hold your maximum size string parameter +1.

stevenmess2004 · 2010-01-07 11:17

PUB Set_Local_Name (name) | len, ptr, temp
  len := strsize(name)
  byte[noparse][[/noparse] temp++] := strsize(name)
  Repeat ptr from 0 to strsize(name)
    byte[noparse][[/noparse] temp++] := byte[noparse][[/noparse] name++]
  'byte[noparse][[/noparse] temp++] := $00
  sendREQ (GAP_WRITE_LOCAL_NAME, temp, len)

When you do a "byte[noparse][[/noparse] value]:=something" spin writes "something" to the byte of memory that "value" points to. In your example "temp" is never set to any particular value so it could be anything and trample over something important. What you need is something more like this.

PUB Set_Local_Name(name)
  sendREQ(GAP_WRITE_LOCAL_NAME,name,strsize(name))

There are a couple of other problems but since this is for your HSC you can have a go at finding them yourself first... What's here should be enough to get you started.

computer guy · 2010-01-07 11:25

This is part of a huge 9 month project Steven and when I purchased the Easy Bluetooth module I expected there to be an object in OBEX for it.
I didn't expect to have to write one from scratch.

Any help would be greatly appreciated as I have already spent a week on it and it is consuming a lot of time.

Thank you

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
"powered by Propeller" domed stickers $1.50 - Find them here
Check out my Design and Technology project for my Higher School Certificate www.ecosureblog.net

computer guy · 2010-01-07 12:43

Well Steven, it's 11:43PM and I just noticed I was missing the clock frequency constants. Now the baud rate is working properly.
Just a few more bugs/stupid errors to fix and I think all will be fine.

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
"powered by Propeller" domed stickers $1.50 - Find them here
Check out my Design and Technology project for my Higher School Certificate www.ecosureblog.net

computer guy · 2010-01-07 14:44

I have modified this code and it's now working, however sendREQ is still not working.

PUB Set_Local_Name (name) | len, ptr, i

  len := strsize(name)
  msgBuffer[noparse][[/noparse]0] := strsize(name)
  Repeat ptr from 1 to len + 1
    msgBuffer[noparse][[/noparse]ptr] := byte[noparse][[/noparse]name][noparse][[/noparse]ptr - 1]
  sendREQ(GAP_WRITE_LOCAL_NAME, msgBuffer, len)

Edit: I'm stumped as to what is wrong.

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
"powered by Propeller" domed stickers $1.50 - Find them here
Check out my Design and Technology project for my Higher School Certificate www.ecosureblog.net

Post Edited (computer guy) : 1/7/2010 3:22:27 PM GMT

StefanL38 · 2010-01-07 16:31

Hi computerguy,

if you want to analyse what is going on simply add a debugoutput for EVERY single step and every single variable that you are using.
This might look like great effort, but it brings you forward step by step. If you do quickshots by changing the code here changing it there
with GUESSING "maybe it works this way" but not REALLY KNOWING if it works this way. This might give a feeling of moving forward. But that's not true
Your just moving around in circles.

computerguy said...
Just a few more bugs/stupid errors to fix and I think all will be fine.

what do you think will be faster?

repeat 499
  Just a few more bugs/stupid errors to fix and I THINK all will be fine.

  repeat debug_line from 1 to 30 
    analyse_whats_going_on

best regards

Stefan

computer guy · 2010-01-07 21:42

Hi Stefan,

Thank you. I was outputing variables to a debug line for debugging.
It helps greatly.

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
"powered by Propeller" domed stickers $1.50 - Find them here
Check out my Design and Technology project for my Higher School Certificate www.ecosureblog.net

computer guy · 2010-01-07 23:20

I got it working!!!! There is two extra $00 bytes being sent, however this isn't interfering as they are after the End Delimiter. Will look at the cause of them later.
Will post the code later, as I would like someone to take a look at it and see if it can be shortened or neatened up a bit.

Thank you

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
"powered by Propeller" domed stickers $1.50 - Find them here
Check out my Design and Technology project for my Higher School Certificate www.ecosureblog.net

computer guy · 2010-01-08 02:23

I have attached a text file copy of both files.
Please note that I have renamed "Full_Duplex_Serial" to "Full_Serial".

Can someone please take a look and see if it can be made smaller or neater (less code, same function)

Thank you

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
"powered by Propeller" domed stickers $1.50 - Find them here
Check out my Design and Technology project for my Higher School Certificate www.ecosureblog.net

stevenmess2004 · 2010-01-08 06:23

Have a look at the attached file.

I've put comments in there in some places that need fixing or can be speed up. It's not tested so I suggest you make the changes one at a time incase I made a typo...

computer guy · 2010-01-08 07:28

Hi Steven,

Thank you for your comments, however the "movebyte" command is causing the entire packet to be screwed up and the activity light on the Easy Bluetooth module is going crazy.
Any Suggestions?

Thank you

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
"powered by Propeller" domed stickers $1.50 - Find them here
Check out my Design and Technology project for my Higher School Certificate www.ecosureblog.net

stevenmess2004 · 2010-01-08 07:53

Try this

[b]PUB[/b] sendREQ (opCode, msg, length) | dataLen

  [b]if[/b] Started
    dataLen := length + 1
    dataSet[noparse][[/noparse]&#173;0&#093; := StartDelimiter
    dataSet[noparse][[/noparse]&#173;1&#093; := REQ_PACKET
    dataSet[noparse][[/noparse]&#173;2&#093; := opCode
    dataSet[noparse][[/noparse]&#173;3&#093; := dataLen
    dataSet[noparse][[/noparse]&#173;4&#093; := dataLen >> 8
    dataSet[noparse][[/noparse]&#173;5&#093; := $00 
    [b]bytemove[/b](@dataSet[noparse][[/noparse]&#173;6&#093;,msg,length+1)   'added the +1 because of the extra bit you add in set_local_name
    dataSet[noparse][[/noparse]&#173;6+length+1+1&#093; := EndDelimiter  
    send_cmd_receive_response(6+length+1+1+1) 'total length is 1 longer than the last address

You should also ignore the last two lines of comments I put in the file. I missed the extra byte you added to the front of the message...

computer guy · 2010-01-08 09:07

Thanks Steven.

It's working well and implements most of your suggestions.
Had to make a few changes to get your changes working properly, nothing major though.

I have attached a copy of what I have.

Thank you

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
"powered by Propeller" domed stickers $1.50 - Find them here
Check out my Design and Technology project for my Higher School Certificate www.ecosureblog.net

SamMishal · 2010-01-08 09:50

Computer Guy,

I like your name...

I looked at your code and I THINK you can reduce some of it and there was one place where
there was some error....I commented all my changes and wrote my reasons.

I hope this helps

....oops you must have been posting while I was posting....well...look at my suggestions
and see if you find any further ideas.....

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
Samuel

www.RobotBASIC.com

SamMishal · 2010-01-08 09:56

Computer Guy,

This method in your UPDATED code will cause TROUBLE....

Also see my code for FURTHER suggestions that I THINK you will benefit from....

PUB receive_response | [color=red]read_data[/color], num_bytes, i
  num_bytes := 0
  i := 0
  repeat
    num_bytes++
    [color=red]read_data[noparse][[/noparse]i] := serial.rx
[/color]    i++
  while (serial.rxcheck > 0)
  [color=red]return read_data[/color]

The red stuff WILL cause memory overwrite and returning the value of the first element of the
non-existing array which is actually an overwrite of memory that may have been code and would be causing
TONS of trouble..see my code for a suggested repaired code.

·

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
Samuel

www.RobotBASIC.com

Post Edited (SamMishal) : 1/8/2010 11:14:41 AM GMT

computer guy · 2010-01-08 11:04

Thank you Sam and Steven

I am fairly happy with the code now. A few commments and it will look professional. :-D
Now to work on the rest of the commands and then the code to interpret the response.

Wish me luck.

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
"powered by Propeller" domed stickers $1.50 - Find them here
Check out my Design and Technology project for my Higher School Certificate www.ecosureblog.net

SamMishal · 2010-01-08 11:15

computer guy said...· Wish me luck.

Good Luck....

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
Samuel

www.RobotBASIC.com
·

computer guy · 2010-01-08 12:14

receive_response() is only returning

$02, $00

When I am expecting something like

$02, $43, $04, $01, $00, $48, $00, $03

The code I am using is

PUB receive_response | i

  i := 0
  repeat
    read_data[noparse][[/noparse]i++] := serial.rx
  while (serial.rxcheck <> -1)

Any ideas?

Thank you

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
"powered by Propeller" domed stickers $1.50 - Find them here
Check out my Design and Technology project for my Higher School Certificate www.ecosureblog.net

SamMishal · 2010-01-08 13:56

Computer Guy,

PUB receive_response | i

  i := 0
  repeat
    read_data[noparse][[/noparse]i++] := serial.rx
  while (serial.rxcheck <> -1)

With the above code it is possible for the RxCheck method to return -1 while the sender is still
sending data due to parallel processing. That is you call the RXCheck method while there is no data
in the buffer but the cog is in the process of receiving.

If you know how many bytes you want to receive and want to wait for them then this is a better code...but
this code will wait FOREVER for the required EXACT number of bytes to be received.

PUB receive_response(n) | i   'do not to forget to modify calling places

  repeat i from 0 to n  'n is expected bytes
    read_data[noparse][[/noparse]i++] := serial.rx

If you do not know the number of bytes or they are not a fixed number then it is better to use a TIMEOUT.

PUB receive_response | i,T

  i := 0
  T := Cnt+TIME_OUT_PERIOD   'define in Constants section as TIME_OUT_PERIOD = ClkFreq/xxxx   xxxx is fractions of 
                             'seconds you need  e.g. for 1 ms then xxxx = 1000 for 30ms then *3/100 etc.
  repeat
    read_data[noparse][[/noparse]i++] := serial.rx
  while (Cnt < T or i =< MAX_BYTES_TO RECEIVE)  'define MAX_BYTES_TO_RECEIVE in the constants section

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
Samuel

www.RobotBASIC.com
·

computer guy · 2010-01-11 22:10

Thank you Samual,

I tried the version with timeout and max_bytes and as long as the max_bytes is equal to, or less greater than what is specified, it works.
However if the number of bytes is less than max_bytes, it sits waiting for the next byte, that never comes. I don't think the timeout code is working.

Any suggestions.

Thank you

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
"powered by Propeller" domed stickers $1.50 - Find them here
Check out my Design and Technology project for my Higher School Certificate www.ecosureblog.net

StefanL38 · 2010-01-11 23:21

look through the FullDuplexSerialPlus-object what timeout methods it offers

best regards

Stefan

SamMishal · 2010-01-12 01:16

Hi Computer Guy,
·
Yes I made a mistake using the Rx method I should have used the RxTime() method.
·
The RxTime() method is the one with the time out feature. here is a correction.....sorry
·

PUB receive_response | i,T
  i := 0
  'define in Constants section as TIME_OUT_PERIOD in Milliseconds


  repeat
    T := read_data[noparse][[/noparse]i++] := serial.RxTime(Time_Out_Period)
  while (T => 0 and i =< Max_Bytes_To_Receive)  'define MAX_BYTES_TO_RECEIVE in the constants section
 [b] return i-1[/b]

·
Note: I have changed the definition of Time_Out_Period.....you should now define it in the constants section
as a MillisSeconds value not as a Ticks ie ClckFreq/xxxxx·.......... make sure it is a Milliseconds....so if you want to wait a maximum
of 10 milliseconds then the definition in the constants section would be Time_Out_Period = 10
·
The above code will keep trying to receive Max_Bytes_To_Receive bytes waiting every time Time_Out_Period milliseconds
to receive each byte. BUT....the first time it fails to receive a byte it will Stop trying to receive.
·
So the Method Receive_Response will try to receive Max_Bytes_To_Receive but will finish when that number of bytes is received or the first
time a byte is not received and there is a time out. The method will return the number of bytes ACTUALLY received.
·
This method is now a lot better than what I had before.....note that you may need to check the return result to see how many bytes
were really received...which currently in your code I am not sure you are doing.
·
Note: I have not tried the method in a real program since I am not near my propeller right now....let me know if it works.
·

P.S.....look at the code again I missed the line in bold...please add it

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
Samuel

www.RobotBASIC.com

Post Edited (SamMishal) : 1/12/2010 1:56:59 AM GMT

SamMishal · 2010-01-12 02:01

Hi Computer Guy,

I have just tried the method (see above...also note I added the line in bold at the bottom of it)
It works as needed...here is a full stand alone program that uses the method to test it.

CON
  _clkmode = xtal1 + pll16x
  _xinfreq = 5_000_000
 
  Max_Bytes_To_Receive = 20
  Time_Out_Period = 2000        '2000 ms because humans need more time
 
Var
  byte read_data[noparse][[/noparse]200]
 
OBJ
  serial    : "FullDuplexSerial"
 
PUB Main|x,i
  Serial.Start(31,30,0,115200)
  waitcnt(ClkFreq+cnt)  'wait a bit to give time to activate the Serial terminal
  x := Receive_Response
  Serial.Tx(16)
  Serial.Dec(x)
  if x
     repeat i from 0 to x-1
        Serial.Tx(13)
        Serial.Dec(Read_Data[noparse][[/noparse]i])

[b]PUB receive_response | i,T
  i := 0
  'define in Constants section as TIME_OUT_PERIOD in Milliseconds[/b]
[b]  repeat
    T := read_data[noparse][[/noparse]i++] := Serial.RxTime(Time_Out_Period)
  while (T => 0 and i =< Max_Bytes_To_Receive)  'define MAX_BYTES_TO_RECEIVE in the constants section
  return i-1
[/b]

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
Samuel

www.RobotBASIC.com
·

computer guy · 2010-01-12 03:32

Thank you Samual,

The code now works great.
I had to modify it a bit as the reset function needs more time, as the module restarts.

I now have

PUB receive_response ([b]ms[/b]) | i,T
  i := 0
  repeat
    T := read_data[noparse][[/noparse]i++] := Serial.RxTime([b]ms[/b])
  while (T => 0 and i =< Max_Bytes_To_Receive)  'define MAX_BYTES_TO_RECEIVE in the constants section
  return i-1

I figured this would be handy to have incase other commands need more than the standard timeout time too.

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
"powered by Propeller" domed stickers $1.50 - Find them here
Check out my Design and Technology project for my Higher School Certificate www.ecosureblog.net

SamMishal · 2010-01-12 05:54

computer guy said...
Thank you Samual, The code now works great.

GREAT....I am glad...you are welcome....

computer guy said...
I had to modify it a bit as the reset function needs more time, as the module restarts.
I figured this would be handy to have incase other commands need more than the standard timeout time too.

Perfect, I am glad that you have done this...it shows that you have REALLY understood its actions·since now you are able to modify it.
Good....I am happy....you can now do even more changes when needed.

Wonderful.........

I always say....give a man a fish and he will eat for a day (well...at least lunch

)....teach him how to fish and he will eventually drown....

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
Samuel

www.RobotBASIC.com
·

computer guy · 2010-02-27 00:55

Hi Samual,

I now need it to receive a packet without sending one first. i.e. An "Incomming connection" packet that indicates that another bluetooth device is wanting to connect.

I am assuming I will need to dedicate a cog to a repeat loop that looks for data and processes it. But how do I read the packet and decide how to react?

Thank you

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
"powered by Propeller" domed stickers $1.50 - Find them here
Check out my Design and Technology project for my Higher School Certificate www.ecosureblog.net

Need code double checked, plz

Comments