<p>At this point it is a bit challenging to conclude what is wrong :)</p>
<p>The other modules, <code>acc_diameter</code>, <code>acc_raduis</code> and <code>acc_json</code> are using the <code>acc_engine</code> interface and the logic in the acc module is slightly different because it is using functions like <code>acc_run_engines</code><br>
In these modules it was assumed that <code>acc_flag</code> and <code>missed_flag</code> should be stored bit shifted, because of the fact  <code>is_eng_acc_on</code> was expecting already bit shifted values.</p>
<p>However the main acc module is not bit shifting them, so this is one thing not making sense.</p>
<p>(Problem A*) Back to normal ACC module, consider the following combination found in <code>acc_logic.c</code> :<br>
<code>flags_to_reset |= log_missed_flag;</code>  +  <code>reset_acc_flag(req, flags_to_reset);</code><br>
<code>log_missed_flag</code> is what ? (depending on the module it can be 1 or 2)<br>
1: bit shifted, then <code>reset_acc_flag</code> is not compatible<br>
<code>#define reset_acc_flag(_rq,_flag) (resetflag((_rq), (_flag)))</code><br>
2: no bit shifted, then we need to use the assignment operator and we can not keep track of multiple flags.<br>
In fact thanks to the review, I just realized this is not good enough for fixing normal ACC module behavior, we need to keep track of multiple flags to reset until the very end of <code>on_missed</code> in case the same flag is used  for <code>db_missed_flag</code> and <code>log_missed_flag</code>, I will revisit my patch.</p>
<p>When I look at <code>acc_mod.c</code> and when I debug the running program.<br>
<code>log_missed_flag</code> and <code>db_missed_flag</code> are not storing bit shifted values, this is why I think we need to align the acc_engine and modules using it with the default behavior.</p>
<p>The main acc module logic will also be fixed at the same time since the code should not always be working well as describer in (A*)</p>
<p>I hope I am not getting confused :)<br>
I will definitely revisit the patch knowing it is not good enough.</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/kamailio/kamailio/pull/1674#issuecomment-429399926">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AF36ZQ-e9hHL63U1Mwcpfwv56uPsImyIks5ukNCNgaJpZM4XYVgP">mute the thread</a>.<img src="https://github.com/notifications/beacon/AF36Zf3YGUzW88LZlq-sMYJZ2st7-v19ks5ukNCNgaJpZM4XYVgP.gif" height="1" width="1" alt="" /></p>
<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/kamailio/kamailio","title":"kamailio/kamailio","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/kamailio/kamailio"}},"updates":{"snippets":[{"icon":"PERSON","message":"@jchavanton in #1674: At this point it is a bit challenging to conclude what is wrong :)\r\n\r\nThe other modules, `acc_diameter`, `acc_raduis` and `acc_json` are using the `acc_engine` interface and the logic in the acc module is slightly different because it is using functions like `acc_run_engines`\r\nIn these modules it was assumed that `acc_flag` and `missed_flag` should be stored bit shifted, because of the fact  `is_eng_acc_on` was expecting already bit shifted values.\r\n\r\nHowever the main acc module is not bit shifting them, so this is one thing not making sense.\r\n\r\n(Problem A*) Back to normal ACC module, consider the following combination found in `acc_logic.c` :\r\n `flags_to_reset |= log_missed_flag;`  +\t`reset_acc_flag(req, flags_to_reset);`\r\n`log_missed_flag` is what ? (depending on the module it can be 1 or 2)\r\n1: bit shifted, then `reset_acc_flag` is not compatible \r\n`#define reset_acc_flag(_rq,_flag)   (resetflag((_rq), (_flag)))`\r\n2: no bit shifted, then we need to use the assignment operator and we can not keep track of multiple flags.\r\nIn fact thanks to the review, I just realized this is not good enough for fixing normal ACC module behavior, we need to keep track of multiple flags to reset until the very end of `on_missed` in case the same flag is used  for `db_missed_flag` and `log_missed_flag`, I will revisit my patch.\r\n\r\nWhen I look at `acc_mod.c` and when I debug the running program.\r\n`log_missed_flag` and `db_missed_flag` are not storing bit shifted values, this is why I think we need to align the acc_engine and modules using it with the default behavior.\r\n\r\nThe main acc module logic will also be fixed at the same time since the code should not always be working well as describer in (A*)\r\n\r\nI hope I am not getting confused :)\r\nI will definitely revisit the patch knowing it is not good enough.\r\n"}],"action":{"name":"View Pull Request","url":"https://github.com/kamailio/kamailio/pull/1674#issuecomment-429399926"}}}</script>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/kamailio/kamailio/pull/1674#issuecomment-429399926",
"url": "https://github.com/kamailio/kamailio/pull/1674#issuecomment-429399926",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
},
{
"@type": "MessageCard",
"@context": "http://schema.org/extensions",
"hideOriginalBody": "false",
"originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB",
"title": "Re: [kamailio/kamailio] acc: generating duplicates (#1674)",
"sections": [
{
"text": "",
"activityTitle": "**Julien Chavanton**",
"activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png",
"activitySubtitle": "@jchavanton",
"facts": [

]
}
],
"potentialAction": [
{
"name": "Add a comment",
"@type": "ActionCard",
"inputs": [
{
"isMultiLine": true,
"@type": "TextInput",
"id": "IssueComment",
"isRequired": false
}
],
"actions": [
{
"name": "Comment",
"@type": "HttpPOST",
"target": "https://api.github.com",
"body": "{\n\"commandName\": \"IssueComment\",\n\"repositoryFullName\": \"kamailio/kamailio\",\n\"issueId\": 1674,\n\"IssueComment\": \"{{IssueComment.value}}\"\n}"
}
]
},
{
"name": "Close pull request",
"@type": "HttpPOST",
"target": "https://api.github.com",
"body": "{\n\"commandName\": \"PullRequestClose\",\n\"repositoryFullName\": \"kamailio/kamailio\",\n\"pullRequestId\": 1674\n}"
},
{
"targets": [
{
"os": "default",
"uri": "https://github.com/kamailio/kamailio/pull/1674#issuecomment-429399926"
}
],
"@type": "OpenUri",
"name": "View on GitHub"
},
{
"name": "Unsubscribe",
"@type": "HttpPOST",
"target": "https://api.github.com",
"body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 392255503\n}"
}
],
"themeColor": "26292E"
}
]</script>