linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH BlueZ] test: Fix payload and model opcode packing in test-mesh
@ 2020-05-14  2:20 Inga Stotland
  2020-05-14 13:54 ` Gix, Brian
  0 siblings, 1 reply; 2+ messages in thread
From: Inga Stotland @ 2020-05-14  2:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

Use correct packing of multi-byte values in message payload bytearray.
For example, a 2-byte opcode 0x8204 is packed as 0x82 0x04, i.e. in
natural order.

Add transaction ID parameter to "set" commands of generic On/Off
model. Server will ignore the identical commands with the same
transaction ID, source and destination during a timeout period
of 6 seconds.
---
 test/test-mesh | 94 ++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 69 insertions(+), 25 deletions(-)

diff --git a/test/test-mesh b/test/test-mesh
index 66055e2de..38f0c0a74 100755
--- a/test/test-mesh
+++ b/test/test-mesh
@@ -146,6 +146,8 @@ APP_VERSION_ID = 0x0001
 
 VENDOR_ID_NONE = 0xffff
 
+TRANSACTION_TIMEOUT = 6
+
 app = None
 bus = None
 mainloop = None
@@ -330,7 +332,7 @@ def print_state(state):
 		print('ON')
 	else:
 		print('UNKNOWN')
-class PubTimer():
+class ModTimer():
 	def __init__(self):
 		self.seconds = None
 		self.func = None
@@ -473,18 +475,18 @@ class Element(dbus.service.Object):
 
 	@dbus.service.method(MESH_ELEMENT_IFACE,
 					in_signature="qqvay", out_signature="")
-	def MessageReceived(self, source, key, destination, data):
+	def MessageReceived(self, source, key, dest, data):
 		print(('Message Received on Element %02x') % self.index, end='')
 		print(', src=', format(source, '04x'), end='')
 
-		if isinstance(destination, int):
-			print(', dst=%04x' % destination)
-		elif isinstance(destination, dbus.Array):
-			dst_str = array_to_string(destination)
+		if isinstance(dest, int):
+			print(', dst=%04x' % dest)
+		elif isinstance(dest, dbus.Array):
+			dst_str = array_to_string(dest)
 			print(', dst=' + dst_str)
 
 		for model in self.models:
-			model.process_message(source, key, data)
+			model.process_message(source, dest, key, data)
 
 	@dbus.service.method(MESH_ELEMENT_IFACE,
 					in_signature="qa{sv}", out_signature="")
@@ -528,7 +530,7 @@ class Model():
 	def get_vendor(self):
 		return self.vendor
 
-	def process_message(self, source, key, data):
+	def process_message(self, source, dest, key, data):
 		return
 
 	def set_publication(self, period):
@@ -576,6 +578,9 @@ class Model():
 class OnOffServer(Model):
 	def __init__(self, model_id):
 		Model.__init__(self, model_id)
+		self.tid = None
+		self.last_src = 0x0000
+		self.last_dst = 0x0000
 		self.cmd_ops = { 0x8201,  # get
 				 0x8202,  # set
 				 0x8203,  # set unacknowledged
@@ -584,48 +589,74 @@ class OnOffServer(Model):
 		print("OnOff Server ")
 		self.state = 0
 		print_state(self.state)
-		self.timer = PubTimer()
+		self.pub_timer = ModTimer()
+		self.t_timer = ModTimer()
 
-	def process_message(self, source, key, data):
+	def process_message(self, source, dest, key, data):
 		datalen = len(data)
 
-		if datalen != 2 and datalen != 3:
+		if datalen != 2 and datalen != 4:
 			# The opcode is not recognized by this model
 			return
 
 		if datalen == 2:
-			op_tuple=struct.unpack('<H',bytes(data))
+			op_tuple=struct.unpack('>H',bytes(data))
 			opcode = op_tuple[0]
+
 			if opcode != 0x8201:
 				# The opcode is not recognized by this model
 				return
 			print('Get state')
-		elif datalen == 3:
-			opcode,self.state=struct.unpack('<HB',bytes(data))
+		elif datalen == 4:
+			opcode,self.state, tid = struct.unpack('>HBB',
+							       bytes(data))
+
 			if opcode != 0x8202 and opcode != 0x8203:
 				# The opcode is not recognized by this model
 				return
 			print_state(self.state)
 
-		rsp_data = struct.pack('<HB', 0x8204, self.state)
+			if (self.tid != None and self.tid == tid and
+						self.last_src == source and
+						self.last_dst == dest):
+				# Ignore duplicate transaction
+				return
+
+			self.t_timer.cancel()
+			self.tid = tid
+			self.last_src = source
+			self.last_dst = dest
+			self.t_timer.start(TRANSACTION_TIMEOUT, self.t_track)
+
+			# Unacknowledged "set"
+			if opcode == 0x8203:
+				return
+
+		rsp_data = struct.pack('>HB', 0x8204, self.state)
 		self.send_message(source, key, rsp_data)
 
+	def t_track(self):
+			self.t_timer.cancel()
+			self.tid = None
+			self.last_src = 0x0000
+			self.last_dst = 0x0000
+
 	def set_publication(self, period):
 
 		self.pub_period = period
 		if period == 0:
-			self.timer.cancel()
+			self.pub_timer.cancel()
 			return
 
 		# We do not handle ms in this example
 		if period < 1000:
 			return
 
-		self.timer.start(period/1000, self.publish)
+		self.pub_timer.start(period/1000, self.publish)
 
 	def publish(self):
 		print('Publish')
-		data = struct.pack('<HB', 0x8204, self.state)
+		data = struct.pack('>HB', 0x8204, self.state)
 		self.send_publication(data)
 
 ########################
@@ -634,6 +665,8 @@ class OnOffServer(Model):
 class OnOffClient(Model):
 	def __init__(self, model_id):
 		Model.__init__(self, model_id)
+		self.tid = 0
+		self.data = None
 		self.cmd_ops = { 0x8201,  # get
 				 0x8202,  # set
 				 0x8203,  # set unacknowledged
@@ -646,16 +679,23 @@ class OnOffClient(Model):
 
 	def get_state(self, dest, key):
 		opcode = 0x8201
-		data = struct.pack('<H', opcode)
-		self._send_message(dest, key, data)
+		self.data = struct.pack('>H', opcode)
+		self._send_message(dest, key, self.data)
 
 	def set_state(self, dest, key, state):
 		opcode = 0x8202
 		print('Set state:', state)
-		data = struct.pack('<HB', opcode, state)
-		self._send_message(dest, key, data)
+		self.data = struct.pack('>HBB', opcode, state, self.tid)
+		self.tid = (self.tid + 1) % 255
+		self._send_message(dest, key, self.data)
+
+	def repeat(self, dest, key):
+		if self.data != None:
+			self._send_message(dest, key, self.data)
+		else:
+			print('No previous command stored')
 
-	def process_message(self, source, key, data):
+	def process_message(self, source, dest, key, data):
 		print('OnOffClient process message len = ', end = '')
 		datalen = len(data)
 		print(datalen)
@@ -664,7 +704,7 @@ class OnOffClient(Model):
 			# The opcode is not recognized by this model
 			return
 
-		opcode, state = struct.unpack('<HB',bytes(data))
+		opcode, state = struct.unpack('>HB',bytes(data))
 
 		if opcode != 0x8204 :
 			# The opcode is not recognized by this model
@@ -919,12 +959,14 @@ class ClientMenu(Menu):
 						self.__cmd_set_state_off),
 			'on': MenuItem(' - set state ON',
 						self.__cmd_set_state_on),
+			'repeat': MenuItem(' - repeat last command',
+						self.__cmd_repeat_transaction),
 			'back': MenuItem(' - back to main menu',
 						self.__cmd_main_menu),
 			'quit': MenuItem(' - exit the test', app_exit)
 		}
 
-		Menu.__init__(self, 'On/Off Clien Menu', menu_items)
+		Menu.__init__(self, 'On/Off Client Menu', menu_items)
 
 	def __cmd_main_menu(self):
 		switch_menu(MAIN_MENU)
@@ -938,6 +980,8 @@ class ClientMenu(Menu):
 	def __cmd_set_state_on(self):
 		app.elements[1].models[0].set_state(dst_addr, app_idx, 1)
 
+	def __cmd_repeat_transaction(self):
+		app.elements[1].models[0].repeat(dst_addr, app_idx)
 
 def set_value(str_value, min, max):
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH BlueZ] test: Fix payload and model opcode packing in test-mesh
  2020-05-14  2:20 [PATCH BlueZ] test: Fix payload and model opcode packing in test-mesh Inga Stotland
@ 2020-05-14 13:54 ` Gix, Brian
  0 siblings, 0 replies; 2+ messages in thread
From: Gix, Brian @ 2020-05-14 13:54 UTC (permalink / raw)
  To: linux-bluetooth, Stotland, Inga; +Cc: testtgsh

Applied
On Wed, 2020-05-13 at 19:20 -0700, Inga Stotland wrote:
> Use correct packing of multi-byte values in message payload bytearray.
> For example, a 2-byte opcode 0x8204 is packed as 0x82 0x04, i.e. in
> natural order.
> 
> Add transaction ID parameter to "set" commands of generic On/Off
> model. Server will ignore the identical commands with the same
> transaction ID, source and destination during a timeout period
> of 6 seconds.
> ---
>  test/test-mesh | 94 ++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 69 insertions(+), 25 deletions(-)
> 
> diff --git a/test/test-mesh b/test/test-mesh
> index 66055e2de..38f0c0a74 100755
> --- a/test/test-mesh
> +++ b/test/test-mesh
> @@ -146,6 +146,8 @@ APP_VERSION_ID = 0x0001
>  
>  VENDOR_ID_NONE = 0xffff
>  
> +TRANSACTION_TIMEOUT = 6
> +
>  app = None
>  bus = None
>  mainloop = None
> @@ -330,7 +332,7 @@ def print_state(state):
>  		print('ON')
>  	else:
>  		print('UNKNOWN')
> -class PubTimer():
> +class ModTimer():
>  	def __init__(self):
>  		self.seconds = None
>  		self.func = None
> @@ -473,18 +475,18 @@ class Element(dbus.service.Object):
>  
>  	@dbus.service.method(MESH_ELEMENT_IFACE,
>  					in_signature="qqvay", out_signature="")
> -	def MessageReceived(self, source, key, destination, data):
> +	def MessageReceived(self, source, key, dest, data):
>  		print(('Message Received on Element %02x') % self.index, end='')
>  		print(', src=', format(source, '04x'), end='')
>  
> -		if isinstance(destination, int):
> -			print(', dst=%04x' % destination)
> -		elif isinstance(destination, dbus.Array):
> -			dst_str = array_to_string(destination)
> +		if isinstance(dest, int):
> +			print(', dst=%04x' % dest)
> +		elif isinstance(dest, dbus.Array):
> +			dst_str = array_to_string(dest)
>  			print(', dst=' + dst_str)
>  
>  		for model in self.models:
> -			model.process_message(source, key, data)
> +			model.process_message(source, dest, key, data)
>  
>  	@dbus.service.method(MESH_ELEMENT_IFACE,
>  					in_signature="qa{sv}", out_signature="")
> @@ -528,7 +530,7 @@ class Model():
>  	def get_vendor(self):
>  		return self.vendor
>  
> -	def process_message(self, source, key, data):
> +	def process_message(self, source, dest, key, data):
>  		return
>  
>  	def set_publication(self, period):
> @@ -576,6 +578,9 @@ class Model():
>  class OnOffServer(Model):
>  	def __init__(self, model_id):
>  		Model.__init__(self, model_id)
> +		self.tid = None
> +		self.last_src = 0x0000
> +		self.last_dst = 0x0000
>  		self.cmd_ops = { 0x8201,  # get
>  				 0x8202,  # set
>  				 0x8203,  # set unacknowledged
> @@ -584,48 +589,74 @@ class OnOffServer(Model):
>  		print("OnOff Server ")
>  		self.state = 0
>  		print_state(self.state)
> -		self.timer = PubTimer()
> +		self.pub_timer = ModTimer()
> +		self.t_timer = ModTimer()
>  
> -	def process_message(self, source, key, data):
> +	def process_message(self, source, dest, key, data):
>  		datalen = len(data)
>  
> -		if datalen != 2 and datalen != 3:
> +		if datalen != 2 and datalen != 4:
>  			# The opcode is not recognized by this model
>  			return
>  
>  		if datalen == 2:
> -			op_tuple=struct.unpack('<H',bytes(data))
> +			op_tuple=struct.unpack('>H',bytes(data))
>  			opcode = op_tuple[0]
> +
>  			if opcode != 0x8201:
>  				# The opcode is not recognized by this model
>  				return
>  			print('Get state')
> -		elif datalen == 3:
> -			opcode,self.state=struct.unpack('<HB',bytes(data))
> +		elif datalen == 4:
> +			opcode,self.state, tid = struct.unpack('>HBB',
> +							       bytes(data))
> +
>  			if opcode != 0x8202 and opcode != 0x8203:
>  				# The opcode is not recognized by this model
>  				return
>  			print_state(self.state)
>  
> -		rsp_data = struct.pack('<HB', 0x8204, self.state)
> +			if (self.tid != None and self.tid == tid and
> +						self.last_src == source and
> +						self.last_dst == dest):
> +				# Ignore duplicate transaction
> +				return
> +
> +			self.t_timer.cancel()
> +			self.tid = tid
> +			self.last_src = source
> +			self.last_dst = dest
> +			self.t_timer.start(TRANSACTION_TIMEOUT, self.t_track)
> +
> +			# Unacknowledged "set"
> +			if opcode == 0x8203:
> +				return
> +
> +		rsp_data = struct.pack('>HB', 0x8204, self.state)
>  		self.send_message(source, key, rsp_data)
>  
> +	def t_track(self):
> +			self.t_timer.cancel()
> +			self.tid = None
> +			self.last_src = 0x0000
> +			self.last_dst = 0x0000
> +
>  	def set_publication(self, period):
>  
>  		self.pub_period = period
>  		if period == 0:
> -			self.timer.cancel()
> +			self.pub_timer.cancel()
>  			return
>  
>  		# We do not handle ms in this example
>  		if period < 1000:
>  			return
>  
> -		self.timer.start(period/1000, self.publish)
> +		self.pub_timer.start(period/1000, self.publish)
>  
>  	def publish(self):
>  		print('Publish')
> -		data = struct.pack('<HB', 0x8204, self.state)
> +		data = struct.pack('>HB', 0x8204, self.state)
>  		self.send_publication(data)
>  
>  ########################
> @@ -634,6 +665,8 @@ class OnOffServer(Model):
>  class OnOffClient(Model):
>  	def __init__(self, model_id):
>  		Model.__init__(self, model_id)
> +		self.tid = 0
> +		self.data = None
>  		self.cmd_ops = { 0x8201,  # get
>  				 0x8202,  # set
>  				 0x8203,  # set unacknowledged
> @@ -646,16 +679,23 @@ class OnOffClient(Model):
>  
>  	def get_state(self, dest, key):
>  		opcode = 0x8201
> -		data = struct.pack('<H', opcode)
> -		self._send_message(dest, key, data)
> +		self.data = struct.pack('>H', opcode)
> +		self._send_message(dest, key, self.data)
>  
>  	def set_state(self, dest, key, state):
>  		opcode = 0x8202
>  		print('Set state:', state)
> -		data = struct.pack('<HB', opcode, state)
> -		self._send_message(dest, key, data)
> +		self.data = struct.pack('>HBB', opcode, state, self.tid)
> +		self.tid = (self.tid + 1) % 255
> +		self._send_message(dest, key, self.data)
> +
> +	def repeat(self, dest, key):
> +		if self.data != None:
> +			self._send_message(dest, key, self.data)
> +		else:
> +			print('No previous command stored')
>  
> -	def process_message(self, source, key, data):
> +	def process_message(self, source, dest, key, data):
>  		print('OnOffClient process message len = ', end = '')
>  		datalen = len(data)
>  		print(datalen)
> @@ -664,7 +704,7 @@ class OnOffClient(Model):
>  			# The opcode is not recognized by this model
>  			return
>  
> -		opcode, state = struct.unpack('<HB',bytes(data))
> +		opcode, state = struct.unpack('>HB',bytes(data))
>  
>  		if opcode != 0x8204 :
>  			# The opcode is not recognized by this model
> @@ -919,12 +959,14 @@ class ClientMenu(Menu):
>  						self.__cmd_set_state_off),
>  			'on': MenuItem(' - set state ON',
>  						self.__cmd_set_state_on),
> +			'repeat': MenuItem(' - repeat last command',
> +						self.__cmd_repeat_transaction),
>  			'back': MenuItem(' - back to main menu',
>  						self.__cmd_main_menu),
>  			'quit': MenuItem(' - exit the test', app_exit)
>  		}
>  
> -		Menu.__init__(self, 'On/Off Clien Menu', menu_items)
> +		Menu.__init__(self, 'On/Off Client Menu', menu_items)
>  
>  	def __cmd_main_menu(self):
>  		switch_menu(MAIN_MENU)
> @@ -938,6 +980,8 @@ class ClientMenu(Menu):
>  	def __cmd_set_state_on(self):
>  		app.elements[1].models[0].set_state(dst_addr, app_idx, 1)
>  
> +	def __cmd_repeat_transaction(self):
> +		app.elements[1].models[0].repeat(dst_addr, app_idx)
>  
>  def set_value(str_value, min, max):
>  

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-05-14 13:55 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-14  2:20 [PATCH BlueZ] test: Fix payload and model opcode packing in test-mesh Inga Stotland
2020-05-14 13:54 ` Gix, Brian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).