Improve color detection for typical use cases #421
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sets the stage for the next commits. This does not change any color detection code yet.
This didn't work great for both sensor types, so move them to their own variants. This still needs to be revisited properly, but this is less bad than before. Also make the h adjustment monotonic so it doesn't skip ahead.
Member
Author
|
Tagging @Novakasa: so this keeps your bicone distance function, but only applies it if the user provides custom colors, which is when it works best. What do you think? |
The bicone mapping is highly distant dependent, which makes it suitable in limited cases, and it doesn't work great with the default colors. If only saturated or grayscale colors are in the mapping, we can generally get a much better result by looking at hue only for saturated colors and looking at value only for unsaturated colors, and use the saturation to decide which to pick. This also means we won't need the workaround of having negative V for None. The logic here is that if you do specify fine-grained, calibrated colors, then it will use the original bicone distance mapping.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The bicone mapping introduced in #104 works quite well in controlled circumstances and fixed distances, but less so in other cases.
For example, the edge of black and white may be seen as green or blue depending on the distance when it happens to be closer according to the distance function. See e.g. https://github.com/orgs/pybricks/discussions/1760, which has been bothering me for some time 😄
In many cases, we do have extra information that we can use. The bicone cost function is a measure for the distance between
HSVaandHSVb, but there is additional insight we can get from considering all candidate colors. Often we have a set of idealized color (e.g. pure red or yellow) and idealized grayscale colors (none, black, or white). One strategy we can apply then is to pick the nearest hue if the measured saturation is high, or otherwise pick the nearest value.This PR applies that strategy when suitable candidate colors are given. Otherwise it defaults to the bicone function. So people who calibrated their own colors can continue using that strategy.
This also lets us undo the workaround of using negative value to get black/none to work at all.
These pictures show the improvement in range. Left (prime) is stable firmware and right (essential) is this PR. The bricks are moved towards the sensor until the hub lights up with the detected color.