<p><a href="https://github.com/kyc0o" class="user-mention">@kYc0o</a> Why did this got merged so fast!!!???? Almost no time for review (3h time window).</p>
<p>IMO this complicates definition of peripheral in the "new" style using structs and will fill the code of <code>#if</code> <code>#else</code>. See <a href="https://github.com/RIOT-OS/RIOT/pull/7542" class="issue-link js-issue-link" data-url="https://github.com/RIOT-OS/RIOT/issues/7542" data-id="254091955" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#7542</a> as an example.</p>
<p>Now four more defines are required:</p>
<div class="highlight highlight-source-c"><pre>#<span class="pl-k">define</span> <span class="pl-en">power_pwm0_enable</span> power_timer0_enable 
#<span class="pl-k">define</span> <span class="pl-en">power_pwm0_disable</span> power_timer0_disable
#<span class="pl-k">define</span> <span class="pl-en">power_pwm1_enable</span> power_timer1_enable 
#<span class="pl-k">define</span> <span class="pl-en">power_pwm1_disable</span> power_timer1_disable</pre></div>
<p>And they will have to be rewritten every time the struct changes. Furthermore, it requires now extra code:</p>
<div class="highlight highlight-source-c"><pre><span class="pl-k">if</span> (dev == <span class="pl-c1">0</span>) {
    <span class="pl-c1">power_pwm0_enable</span>();
}
<span class="pl-k">else</span> <span class="pl-k">if</span> (dev == <span class="pl-c1">1</span>) {
    <span class="pl-c1">power_pwm1_enable</span>();
}</pre></div>
<p>And this will happen for every single peripheral defined in the "new" style if we do not want to list all of them. I'm not sure this is an advantage in all the situations.</p>
<p>I disagree with part of this PR and suggest to revert the merge. The functions should be used whenever possible but should not be a must. Makes no sense. Besides they are simple defines which do the same thing.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/RIOT-OS/RIOT/pull/7601#issuecomment-329566847">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AEn7YCEC_kl1jtADls1o0356kB3unNNLks5siW4ugaJpZM4PXP30">mute the thread</a>.<img alt="" height="1" src="https://github.com/notifications/beacon/AEn7YJstMY8bzYonxfcbrqHwX2Qescuuks5siW4ugaJpZM4PXP30.gif" width="1" /></p>
<div itemscope itemtype="http://schema.org/EmailMessage">
<div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
  <link itemprop="url" href="https://github.com/RIOT-OS/RIOT/pull/7601#issuecomment-329566847"></link>
  <meta itemprop="name" content="View Pull Request"></meta>
</div>
<meta itemprop="description" content="View this Pull Request on GitHub"></meta>
</div>

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/RIOT-OS/RIOT","title":"RIOT-OS/RIOT","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/RIOT-OS/RIOT"}},"updates":{"snippets":[{"icon":"PERSON","message":"@lebrush in #7601: @kYc0o Why did this got merged so fast!!!???? Almost no time for review (3h time window). \r\n\r\nIMO this complicates definition of peripheral in the \"new\" style using structs and will fill the code of `#if` `#else`. See #7542 as an example.\r\n\r\nNow four more defines are required:\r\n\r\n```C\r\n#define power_pwm0_enable power_timer0_enable \r\n#define power_pwm0_disable power_timer0_disable\r\n#define power_pwm1_enable power_timer1_enable \r\n#define power_pwm1_disable power_timer1_disable\r\n```\r\n\r\nAnd they will have to be rewritten every time the struct changes. Furthermore, it requires now extra code:\r\n\r\n\r\n```C\r\n\r\nif (dev == 0) {\r\n    power_pwm0_enable();\r\n}\r\nelse if (dev == 1) {\r\n    power_pwm1_enable();\r\n}\r\n```\r\n\r\nAnd this will happen for every single peripheral defined in the \"new\" style if we do not want to list all of them. I'm not sure this is an advantage in all the situations.\r\n\r\nI disagree with part of this PR and suggest to revert the merge. The functions should be used whenever possible but should not be a must. Makes no sense. Besides they are simple defines which do the same thing."}],"action":{"name":"View Pull Request","url":"https://github.com/RIOT-OS/RIOT/pull/7601#issuecomment-329566847"}}}</script>