Skip to content

Conversation

microbit-matt-hillsdon
Copy link
Contributor

HAL implementers need to work with both
microbit_hal_level_detector_callback and
microbit_hal_microphone_set_threshold. It's confusing for the former to
use 1 and 2 and the latter 0 and 1 to represent low and high.

Consistently use 1 and 2 via the HAL defines.

HAL implementers need to work with both
microbit_hal_level_detector_callback and
microbit_hal_microphone_set_threshold. It's confusing for the former to
use 1 and 2 and the latter 0 and 1 to represent low and high.

Consistently use 1 and 2 via the HAL defines.
@microbit-carlos microbit-carlos added this to the 2.1 milestone Aug 8, 2022
microbit-matt-hillsdon added a commit to microbit-foundation/micropython-microbit-v2-simulator that referenced this pull request Aug 9, 2022
microbit-matt-hillsdon pushed a commit to microbit-foundation/micropython-microbit-v2-simulator that referenced this pull request Aug 9, 2022
Includes temporary workaround for inconsistent values in the hal to represent thresholds.
See microbit-foundation/micropython-microbit-v2#109
@dpgeorge
Copy link
Collaborator

dpgeorge commented Aug 15, 2022

Instead of this I think we should make two new defines to set the threshold (the existing ones are event numbers), because these two sets are conceptually independent.

Eg:

#define MICROBIT_HAL_MICROPHONE_SET_THRESHOLD_LOW (0)
#define MICROBIT_HAL_MICROPHONE_SET_THRESHOLD_HIGH (1)

And probably rename the existing ones to:

#define MICROBIT_HAL_MICROPHONE_EVT_THRESHOLD_LOW (1)
#define MICROBIT_HAL_MICROPHONE_EVT_THRESHOLD_HIGH (2)

@microbit-carlos microbit-carlos modified the milestones: 2.1-beta.1, 2.1.0 Aug 26, 2022
@dpgeorge
Copy link
Collaborator

As discussed elsewhere, I implemented my above suggestion in a1be53a and 804c5fe

@dpgeorge dpgeorge closed this Aug 31, 2022
@dpgeorge dpgeorge deleted the microphone-hal-tweak branch August 31, 2022 11:27
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.

3 participants