diff --git a/idtap/classes/meter.py b/idtap/classes/meter.py index 5cd9f2d..4a68775 100644 --- a/idtap/classes/meter.py +++ b/idtap/classes/meter.py @@ -474,9 +474,12 @@ def _hierarchical_position_to_pulse_index(self, positions: List[int], cycle_numb def _pulse_index_to_hierarchical_position(self, pulse_index: int, cycle_number: int) -> List[int]: """Convert pulse index back to hierarchical position (reverse of _hierarchical_position_to_pulse_index).""" - # Remove cycle offset - cycle_offset = cycle_number * self._pulses_per_cycle - within_cycle_index = pulse_index - cycle_offset + # Use modulo to get within-cycle index regardless of which cycle the pulse belongs to + within_cycle_index = pulse_index % self._pulses_per_cycle + + # Ensure within_cycle_index is non-negative + if within_cycle_index < 0: + within_cycle_index = 0 positions = [] remaining_index = within_cycle_index @@ -496,9 +499,9 @@ def _pulse_index_to_hierarchical_position(self, pulse_index: int, cycle_number: inner_size = sum(inner_size) group_size = group_size // inner_size - position_at_level = remaining_index // group_size + position_at_level = remaining_index // group_size if group_size > 0 else 0 positions.append(position_at_level) - remaining_index = remaining_index % group_size + remaining_index = remaining_index % group_size if group_size > 0 else 0 return positions @@ -561,8 +564,30 @@ def get_musical_time(self, real_time: float, reference_level: Optional[int] = No if real_time < self.start_time: return False - end_time = self.start_time + self.repetitions * self.cycle_dur - if real_time >= end_time: + # Calculate proper end time based on actual pulse timing + # For intermediate cycles: use actual next cycle start pulse + # For final cycle: use theoretical calculation (no next cycle exists) + if self.all_pulses and len(self.all_pulses) > 0: + # Calculate which cycle this time would fall into + relative_time = real_time - self.start_time + potential_cycle = int(relative_time // self.cycle_dur) + + if potential_cycle < self.repetitions - 1: + # This is an intermediate cycle - use actual next cycle start pulse + next_cycle_first_pulse_index = (potential_cycle + 1) * self._pulses_per_cycle + if next_cycle_first_pulse_index < len(self.all_pulses): + actual_end_time = self.all_pulses[next_cycle_first_pulse_index].real_time + else: + # Fallback to theoretical if pulse doesn't exist + actual_end_time = self.start_time + self.repetitions * self.cycle_dur + else: + # This is the final cycle - use theoretical end time + actual_end_time = self.start_time + self.repetitions * self.cycle_dur + else: + # No pulses available - use theoretical calculation + actual_end_time = self.start_time + self.repetitions * self.cycle_dur + + if real_time > actual_end_time: return False # Validate reference level diff --git a/idtap/tests/musical_time_test.py b/idtap/tests/musical_time_test.py index 402c634..4ffad2f 100644 --- a/idtap/tests/musical_time_test.py +++ b/idtap/tests/musical_time_test.py @@ -164,8 +164,11 @@ def test_boundary_conditions(self): result = meter.get_musical_time(end_time - 0.01) assert result is not False - # At or after end - assert meter.get_musical_time(end_time) is False + # At end (should be valid after Issue #38 fix) + result = meter.get_musical_time(end_time) + assert result is not False, "Exact end time should be valid (Issue #38 fix)" + + # After end (should still be invalid) assert meter.get_musical_time(end_time + 0.01) is False def test_reference_level_validation(self): @@ -844,3 +847,42 @@ def test_issue_36_hierarchical_position_correction(self): pulse_time = meter.all_pulses[pulse_index].real_time assert pulse_time <= time, f"Calculated pulse should come at/before query time {time}, got {pulse_time}" + + def test_issue_38_cycle_boundary_failures(self): + """Test fix for Issue #38: get_musical_time() fails at cycle boundaries. + + Ensures that get_musical_time() returns valid musical time objects for + timestamps at exact cycle boundaries, including the final meter boundary. + """ + # Create meter with multiple cycles to test all boundary types + meter = Meter(hierarchy=[4, 4, 2], start_time=0.0, tempo=60.0, repetitions=4) + + # Test each cycle boundary including the final one + for cycle in range(meter.repetitions + 1): + boundary_time = meter.start_time + cycle * meter.cycle_dur + + result = meter.get_musical_time(boundary_time) + + # All boundaries should return valid musical time objects + assert result is not False, f"Cycle boundary at {boundary_time} should return valid musical time" + assert 0.0 <= result.fractional_beat < 1.0, f"fractional_beat should be in valid range for boundary {boundary_time}" + + # Boundary should be treated as start of next cycle (if not final boundary) + if cycle < meter.repetitions: + assert result.cycle_number == cycle, f"Boundary {boundary_time} should be in cycle {cycle}" + assert result.hierarchical_position[0] == 0, f"Boundary should be at start of hierarchical position" + else: + # Final boundary - should be treated as start of theoretical next cycle + assert result.cycle_number == cycle, f"Final boundary should indicate cycle {cycle}" + + # Test times very close to boundaries to ensure they also work + for cycle in range(meter.repetitions): + boundary_time = meter.start_time + (cycle + 1) * meter.cycle_dur + + # Test time just before boundary + near_boundary = boundary_time - 0.001 + result = meter.get_musical_time(near_boundary) + assert result is not False, f"Time just before boundary {boundary_time} should be valid" + + # Should be in the previous cycle + assert result.cycle_number == cycle, f"Time before boundary should be in cycle {cycle}"