Fixes to Decode Event filtering #1295
Conversation
|
Does this also fix #1287 ? |
|
@Nokoa I think it does. Description of #1287 sounds exactly like what I've observed for all decoder types I've tested this with (it's point (5) from this PR's description). This didn't include DMR specifically, but the affected code-paths apply to all decoder types. Feel free to check if it fixes it with your setup 🙂 |
|
At a first glance right now, it appears that DMR Events are appearing in real time. Thank you! |
| @@ -40,7 +40,7 @@ public class DecodeEvent implements IDecodeEvent | |||
| private Protocol mProtocol; | |||
| private Integer mTimeslot; | |||
|
|
|||
| public DecodeEvent(long start) | |||
| protected DecodeEvent(long start) | |||
There was a problem hiding this comment.
this constructor is only used by builders, hence it can be protected
| @@ -332,4 +327,26 @@ public String toString() | |||
| } | |||
| return sb.toString(); | |||
| } | |||
|
|
|||
| @Override | |||
| public boolean equals(Object o) { | |||
There was a problem hiding this comment.
equals and hashCode are for tests
| @@ -44,12 +44,12 @@ dependencies { | |||
|
|
|||
| // JUnit Tests | |||
| testImplementation 'junit:junit:4.12' | |||
| testImplementation "org.junit.jupiter:junit-jupiter-api:5.7.0" | |||
| testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.7.0' | |||
| testRuntimeOnly 'org.junit.vintage:junit-vintage-engine:5.7.0' | |||
There was a problem hiding this comment.
add vintage JUnit engine capable of handling JUnit 4 tests
|
|
||
| import java.util.List; | ||
|
|
||
| public interface CommonFixtures { |
There was a problem hiding this comment.
this will likely evolve into per-entity fixtures. for now it's enough to have a common support interface that can be mixed in to be able to construct basic building blocks with some default values
…ering and adds tests for changed behavior
|
I tested this out a few minutes ago and noticed that the Events message filter now has the Everything Else selection, which all P25 events seem to now fall under. In other words, there's currently no filtering available, it's all or nothing. I do see that you mention this in your first post here, so it's a known issue. Is there an estimated timeline for the events to get properly mapped back to their individual filter selections? Thanks, appreciate the effort! |
|
@GTR8000 part 2 of solving the problems is in #1099. The author had problems with rebasing (see thread in that PR) but with his permission I could help him and (if needed) take it over and finish. In that case I think a realistic timeline would be 4 weeks from now since it's holiday season. If the author doesn't let me finish it (lets me help him with rebasing) I cannot provide any timeline as it's then not really on me. |
|
Closing. Contents of this PR were merged under PR #1601 |
This is my attempt at fixing current implementation of decode event filtering which has the following problems:
Wrong layout with excessive space between button bar and decode event list view:
Current decode event filter set not covering all
DecodeEventTypes -CALLwas missingDecodeEventBuildernot honoringDecodeEventTypeset on itDecode event list ("Events" view) is not properly reversing list of events and filtering it using set filters when changing channels in "Now Playing" view - notice the ordering below before the fix. However, new events are added at the top.
Most of the
DecodeEvents in the codebase do not haveDecodeEventTypeset at all (so it'snull). As a consequence they do not passEventFilter.passesand by extension - they are not added to the "Events" list and do not receive updates (e.g. call duration). This is fixed by adding a new "Everything else" node in the filter tree which acts as a catch-all for all such events. An alternative would be to go over the whole codebase and make sure each createdDecodeEventhas type specified.Any remarks welcome 🙂
Edit: I see that #1099 is a follow-up to the #1098 that introduced event filtering. I think this PR could be an interim solution before #1099 gets merged (and fix issues that are not addressed by #1099)
Edit 2: I started adding some JUnit tests to verify the new behavior, following boy-scout approach. I plan to add tests for most of the changes in this PR.
Edit 3: I added tests following TDD and then adjusted implementation to adhere to the contract specified by the tests.