Skip to content

Add unit tests for CiA 402 profile (coverage 36% → 45%)#644

Open
bizfsc wants to merge 3 commits intocanopen-python:masterfrom
bizfsc:test/p402-coverage
Open

Add unit tests for CiA 402 profile (coverage 36% → 45%)#644
bizfsc wants to merge 3 commits intocanopen-python:masterfrom
bizfsc:test/p402-coverage

Conversation

@bizfsc
Copy link
Copy Markdown
Contributor

@bizfsc bizfsc commented Apr 28, 2026

Summary

Add 24 unit tests for canopen/profiles/p402.py to improve test coverage from 36% to 45%.

Tests Added

  • State402 enum: all states present, statusword decoding via bitmask
  • State decoding: State402.decode_statusword() for all defined states
  • Command words: verify controlword values for every state transition
  • Next-state calculation: walk through the CiA 402 state machine graph
  • Homing status: IS_HOMING / HOMING_COMPLETED / HOMING_ERROR evaluation
  • Operation mode: switching and reading via OD entries 0x6060/0x6061
  • TPDO callback: statusword update propagation through PDO mapping

Approach

Uses a minimal ObjectDictionary with the required DS402 objects (0x6040 controlword, 0x6041 statusword, 0x6060/0x6061 operation mode, 0x6502 supported drives). PDO maps are mocked with unittest.mock.MagicMock to test the TPDO callback path without requiring a real CAN bus.

Coverage

canopen/profiles/p402.py    36% → 45%

The remaining uncovered code is primarily in BaseNode402 methods that require a live CAN bus interaction (SDO reads/writes, NMT state changes, PDO communication).

bizfsc and others added 3 commits April 28, 2026 14:12
Add 24 unit tests covering:
- State402 enum and state decoding from statusword
- Command word generation for state transitions
- Next-state calculation through the state machine
- Homing status evaluation
- Operation mode switching and reading
- TPDO callback handling for statusword updates
- Lookup table consistency checks
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Member

@acolomb acolomb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks nice, but much more valuable would be a simulation of actual TPDO reception.

Comment thread test/test_p402.py
_ = self.node.controlword


class TestBaseNode402NextState(unittest.TestCase):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an internal method, not an API. While it's nice to see some consistency checks on it, the real test subject should be the .state property. I know that's a bit more involved, but injecting TPDO value changes should provide a nice way to simulate a real CAN node answering.

Comment thread test/test_p402.py
Comment on lines +241 to +246
status = None
for key, value in Homing.STATES.items():
bitmask, bits = value
if sw & bitmask == bits:
status = key
self.assertEqual(status, expected)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty useless, as the node itself is not even involved. Again, trying to simulate TPDO reception would be a more valuable approach. As is, this test class could be nuked.

Comment thread test/test_p402.py
Comment on lines +256 to +259
def test_supported_bitmask_unique(self):
values = list(OperationMode.SUPPORTED.values())
# All bitmasks should be unique (each is a single bit)
self.assertEqual(len(values), len(set(values)))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a funky AI idea. The standard defines the bit values, I see no need for testing them against some artificial logic condition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants