Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up EVS ports implementation #93

Closed
skliper opened this issue Sep 30, 2019 · 2 comments · Fixed by #2295
Closed

Clean up EVS ports implementation #93

skliper opened this issue Sep 30, 2019 · 2 comments · Fixed by #2295

Comments

@skliper
Copy link
Contributor

skliper commented Sep 30, 2019

Fix a few minor issues with EVS ports implementation:

  1. Although a macro is used when checking bits of the "OutputPort" mask, the value is still effectively hard coded with the shift, so the macro cannot change without also breaking these checks (violates the spirit of using the macro to begin with). A simplification of the conditional will allow them to change independently (just mask and non-zero):

    /* Process command data */
    if (((CmdPtr->BitMask & CFE_EVS_PORT1_BIT) >> 0) == true)
    {
    CFE_EVS_Global.EVS_TlmPkt.Payload.OutputPort |= CFE_EVS_PORT1_BIT;
    }
    if (((CmdPtr->BitMask & CFE_EVS_PORT2_BIT) >> 1) == true)
    {
    CFE_EVS_Global.EVS_TlmPkt.Payload.OutputPort |= CFE_EVS_PORT2_BIT;
    }
    if (((CmdPtr->BitMask & CFE_EVS_PORT3_BIT) >> 2) == true)
    {
    CFE_EVS_Global.EVS_TlmPkt.Payload.OutputPort |= CFE_EVS_PORT3_BIT;
    }
    if (((CmdPtr->BitMask & CFE_EVS_PORT4_BIT) >> 3) == true)
    {
    CFE_EVS_Global.EVS_TlmPkt.Payload.OutputPort |= CFE_EVS_PORT4_BIT;
    }

    /* Process command data */
    if (((CmdPtr->BitMask & CFE_EVS_PORT1_BIT) >> 0) == true)
    {
    CFE_EVS_Global.EVS_TlmPkt.Payload.OutputPort &= ~CFE_EVS_PORT1_BIT;
    }
    if (((CmdPtr->BitMask & CFE_EVS_PORT2_BIT) >> 1) == true)
    {
    CFE_EVS_Global.EVS_TlmPkt.Payload.OutputPort &= ~CFE_EVS_PORT2_BIT;
    }
    if (((CmdPtr->BitMask & CFE_EVS_PORT3_BIT) >> 2) == true)
    {
    CFE_EVS_Global.EVS_TlmPkt.Payload.OutputPort &= ~CFE_EVS_PORT3_BIT;
    }
    if (((CmdPtr->BitMask & CFE_EVS_PORT4_BIT) >> 3) == true)
    {
    CFE_EVS_Global.EVS_TlmPkt.Payload.OutputPort &= ~CFE_EVS_PORT4_BIT;
    }

    if (((CFE_EVS_Global.EVS_TlmPkt.Payload.OutputPort & CFE_EVS_PORT1_BIT) >> 0) == true)
    {
    /* Copy event message to string format */
    snprintf(PortMessage, CFE_EVS_MAX_PORT_MSG_LENGTH, "EVS Port1 %u/%u/%s %u: %s",
    (unsigned int)EVS_PktPtr->Payload.PacketID.SpacecraftID,
    (unsigned int)EVS_PktPtr->Payload.PacketID.ProcessorID, EVS_PktPtr->Payload.PacketID.AppName,
    (unsigned int)EVS_PktPtr->Payload.PacketID.EventID, EVS_PktPtr->Payload.Message);
    /* Send string event out port #1 */
    EVS_OutputPort1(PortMessage);
    }
    if (((CFE_EVS_Global.EVS_TlmPkt.Payload.OutputPort & CFE_EVS_PORT2_BIT) >> 1) == true)
    {
    /* Copy event message to string format */
    snprintf(PortMessage, CFE_EVS_MAX_PORT_MSG_LENGTH, "EVS Port2 %u/%u/%s %u: %s",
    (unsigned int)EVS_PktPtr->Payload.PacketID.SpacecraftID,
    (unsigned int)EVS_PktPtr->Payload.PacketID.ProcessorID, EVS_PktPtr->Payload.PacketID.AppName,
    (unsigned int)EVS_PktPtr->Payload.PacketID.EventID, EVS_PktPtr->Payload.Message);
    /* Send string event out port #2 */
    EVS_OutputPort2(PortMessage);
    }
    if (((CFE_EVS_Global.EVS_TlmPkt.Payload.OutputPort & CFE_EVS_PORT3_BIT) >> 2) == true)
    {
    /* Copy event message to string format */
    snprintf(PortMessage, CFE_EVS_MAX_PORT_MSG_LENGTH, "EVS Port3 %u/%u/%s %u: %s",
    (unsigned int)EVS_PktPtr->Payload.PacketID.SpacecraftID,
    (unsigned int)EVS_PktPtr->Payload.PacketID.ProcessorID, EVS_PktPtr->Payload.PacketID.AppName,
    (unsigned int)EVS_PktPtr->Payload.PacketID.EventID, EVS_PktPtr->Payload.Message);
    /* Send string event out port #3 */
    EVS_OutputPort3(PortMessage);
    }
    if (((CFE_EVS_Global.EVS_TlmPkt.Payload.OutputPort & CFE_EVS_PORT4_BIT) >> 3) == true)

  2. When sending to multiple ports, the (essentially) same snprintf() is done for each one and only one digit in the string changes. Would be way more efficient to construct the later half of the string once, and change the port number.

The first issue should be resolved prior to integration with EDS as those macros become part of the EDS. Therefore they should be changeable without breaking the implementation that uses them. The second issue is just a performance/size enhancement.

@skliper skliper self-assigned this Sep 30, 2019
@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Imported from trac issue 62. Created by jphickey on 2015-06-04T12:13:01, last modified: 2019-04-03T17:41:37

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jhageman on 2019-04-03 17:41:37:

CCB 4/3/19: See also #94, coordinate solution with implementation of a PSP SendMessageToPort.

@skliper skliper assigned jphickey and unassigned skliper Sep 30, 2019
@skliper skliper changed the title Clean up EVS_SendViaPorts() function Clean up EVS ports implementation Apr 27, 2021
@skliper skliper added the CFS-41 label Apr 27, 2021
skliper added a commit to skliper/cFE that referenced this issue Apr 17, 2023
dzbaker added a commit that referenced this issue Apr 28, 2023
Fix #219, #93, Add EVS port timestamp and simplify port selection
@dzbaker dzbaker added this to the Equuleus milestone Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants