ccollins476ad commented on a change in pull request #836: [WIP] PWM add cycle interrupt handling to pwm_nrf52 driver URL: https://github.com/apache/mynewt-core/pull/836#discussion_r170064494
########## File path: hw/drivers/pwm/pwm_nrf52/src/pwm_nrf52.c ########## @@ -177,9 +249,10 @@ nrf52_pwm_close(struct os_dev *odev) dev = (struct pwm_dev *) odev; inst_id = dev->pwm_instance_id; + assert(instances[inst_id].in_use); Review comment: I think an assert is appropriate for certain failure conditions: * software bugs (i.e., logical contradiction) * unlikely hardware failure In the case of a software bug, my view is that it is best to terminate as soon as possible. This makes it easier to identify and fix the bug, and generally there is no reasonable recovery mechanism. For "safety critical" systems, presumably there are hardware recovery mechanisms or redundancies that protect against a software crash. I am not familiar with this PWM driver, but it looks like this `assert` will only fail if there is a firmware bug. There are similar asserts that check this same condition elsewhere in this file. Anyway, I'm sure opinions vary, so that is just my take. ---------------------------------------------------------------- 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: us...@infra.apache.org With regards, Apache Git Services