[GitHub] wes3 commented on a change in pull request #1432: Add enable/disable SAADC channel to nordic SDK

[GitBox <gi...@apache.org>]

wes3 commented on a change in pull request #1432: Add enable/disable SAADC channel to nordic SDK
URL: https://github.com/apache/mynewt-core/pull/1432#discussion_r221676098
 
 

 ##########
 File path: hw/mcu/nordic/src/ext/nrfx/drivers/src/nrfx_saadc.c
 ##########
 @@ -627,4 +627,20 @@ void nrfx_saadc_limits_set(uint8_t channel, int16_t limit_low, int16_t limit_hig
         nrf_saadc_int_enable(int_mask);
     }
 }
+
+void nrfx_enable_adc_chan(int chan, nrf_saadc_input_t pselp,
+                          nrf_saadc_input_t pseln)
+{
+    NRFX_ASSERT(m_cb.active_channels < NRF_SAADC_CHANNEL_COUNT);
+    ++m_cb.active_channels;
 
 Review comment:
   Hmm... I think it is definitely a worthy comment. I did not consider this when I wrote this code but one thing I did consider was the fact that this function is not thread safe. With these small helper functions I generally do not make them thread safe nor check what you are suggesting as I want to make them as small as possible and the presumption is that the user already knows that the channel is enabled. Thinking about this, I would be more inclined to NRFX_ASSERT() if the channel was not disabled when enabling or enabled when disabling it. I think that would point out some unexpected error. A compromise (so to speak) could be to return an error in that case and let the caller decide if they want to assert but again, this is starting to go against why I wanted to add these in the first place: less time/code spent enabling/disabling a channel by not calling channel_init() and channel_uninit(). I was even considering not checking the # of active channels for sanity when incrementing or decrementing it :-)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services