[v4,0/2] cec: add tests for Give Deck Status message

Message

Deborah Brouwer June 4, 2021, 2:40 a.m. UTC

Changes since v3:
	Patch 1/2:
		- Remove the CEC Version checks and else block.
		- Add comment explaining why it works for CEC 1.4.
	Patch 2/2:
		- Keep/add empty line between variable declarations and code.
		- Remove restriction to playback or recording devices.
		- Extract the deck status where it was previously missed.
		- Extend comment to explain CEC_OP_STATUS_REQ_OFF test.
		- In the invalid test, remove redundant testing.

Changes since v2:
	Patch 1/2 cec-follower: emulate features for CEC versions < CEC 2.0
	* I already sent this as a single patch, and I have not made
		changes since then, I am just including it because now I
		need it for Patch 2/2 to apply.

	Patch 2/2 cec: add tests for Give Deck Status message
	* Combine functions with same name "deck_ctl_give_status"
		to avoid confusion about the naming but also to avoid
		returning FAIL rather than using "fail_on_test" directly.
	* Expand the "Invalid Operand" test so that it will also test a
		follower running version < CEC 2.0.
	* Change the invalid operand from 0xaa to 0 and add a test for
		invalid operand "4", both just outside of the valid range.
	* Expand the "Invalid Operand" test to catch the other reasons
		for which a follower might Feature Abort (e.g. Unrecognized
		Op).

Changes since v1:
	* Remove unnecessary functions.
	* Revise function for turning Give Deck Status reporting On/Off.
	* Combine the Give Deck Status Reporting test into the first
	   Give Deck Status test.

Deborah Brouwer (2):
  cec-follower: emulate features for CEC versions < CEC 2.0
  cec: add tests for Give Deck Status message

 utils/cec-compliance/cec-test.cpp     | 72 ++++++++++++++++++++++-----
 utils/cec-follower/cec-follower.cpp   | 44 +++++++++-------
 utils/cec-follower/cec-follower.h     |  1 +
 utils/cec-follower/cec-processing.cpp | 41 +++++++--------
 utils/libcecutil/cec-info.cpp         |  2 -
 5 files changed, 104 insertions(+), 56 deletions(-)

--
2.17.1

Comments

Hans Verkuil June 4, 2021, 7:53 a.m. UTC | #1

Hi Deb,

Thank you for your work, I've applied the patches.

One note for future patch series: the cover letter should also explain
a bit about what the patch series is about, not just the change
history. Something to keep in mind for the next patch series.

Regards,

	Hans

On 04/06/2021 04:40, Deborah Brouwer wrote:
> Changes since v3:
> 	Patch 1/2:
> 		- Remove the CEC Version checks and else block.
> 		- Add comment explaining why it works for CEC 1.4.
> 	Patch 2/2:
> 		- Keep/add empty line between variable declarations and code.
> 		- Remove restriction to playback or recording devices.
> 		- Extract the deck status where it was previously missed.
> 		- Extend comment to explain CEC_OP_STATUS_REQ_OFF test.
> 		- In the invalid test, remove redundant testing.
> 
> Changes since v2:
> 	Patch 1/2 cec-follower: emulate features for CEC versions < CEC 2.0
> 	* I already sent this as a single patch, and I have not made
> 		changes since then, I am just including it because now I
> 		need it for Patch 2/2 to apply.
> 
> 	Patch 2/2 cec: add tests for Give Deck Status message
> 	* Combine functions with same name "deck_ctl_give_status"
> 		to avoid confusion about the naming but also to avoid
> 		returning FAIL rather than using "fail_on_test" directly.
> 	* Expand the "Invalid Operand" test so that it will also test a
> 		follower running version < CEC 2.0.
> 	* Change the invalid operand from 0xaa to 0 and add a test for
> 		invalid operand "4", both just outside of the valid range.
> 	* Expand the "Invalid Operand" test to catch the other reasons
> 		for which a follower might Feature Abort (e.g. Unrecognized
> 		Op).
> 
> Changes since v1:
> 	* Remove unnecessary functions.
> 	* Revise function for turning Give Deck Status reporting On/Off.
> 	* Combine the Give Deck Status Reporting test into the first
> 	   Give Deck Status test.
> 
> Deborah Brouwer (2):
>   cec-follower: emulate features for CEC versions < CEC 2.0
>   cec: add tests for Give Deck Status message
> 
>  utils/cec-compliance/cec-test.cpp     | 72 ++++++++++++++++++++++-----
>  utils/cec-follower/cec-follower.cpp   | 44 +++++++++-------
>  utils/cec-follower/cec-follower.h     |  1 +
>  utils/cec-follower/cec-processing.cpp | 41 +++++++--------
>  utils/libcecutil/cec-info.cpp         |  2 -
>  5 files changed, 104 insertions(+), 56 deletions(-)
> 
> --
> 2.17.1
>