0
\$\begingroup\$

I have a STM32 microcontroller which implements a special UART protocol which is packet-based. I use FreeRTOS to run different tasks. For instance, I have one task that implements a PID control loop and another task that displays data on a LCD. The control loop task and the "display" task get their parameters (setpoint, PID parameters and so on) from a communication task. And here it becomes interesting. I have three approaches:

a) the communications task handles the entire communication. It receives the data bytes, assembles the packets and, depending on their content, sends the acknowledge packets. Further, the "display" and control loop tasks are notified using messages when new parameters are received. The code becomes more complicated, but on the other hand, less interprocess communication is required since almost all the data handling occurs inside the same task and therefore uses the same context.

b) The communications task only handles the protocol (e.g. decides whether it shall send an acknowledge packet and so on). An additional receive task performs nothing more than just receive all data bytes, assemble the packets and notifies the communications task using event flags what kind of data packet has arrived. Data to be transmitted is sent by the communications task. The advantage of this is that I can do both at the same time, receive data and wait for a specified packet. The disadvantage is that it consumes more RAM and has higher overhead.

c) there are three tasks, one for receiving the data, one for transmitting the data, and one which only handles the protocol. In my opinion this is the cleanest approach, but maybe it is a bit overkill for this application.

Which of these approaches is the best and which one is used probably most often in real-world applications?

Edit: I have provided some C-style pseudocode below to illustrate my problem. See comments below the pseudocode.

pdata = ... /* allocate some memory buffer. */
for(;;)
{
  /* receive_packet blocks until one packet is received. pdata then contains the data. */
  receive_packet(&pdata);

  /* do something with pdata, e.g. change the setpoint for the controller or display some message on the LCD */

  /* for some packets, we need to send an acknowledge. */
  if(ack_needs_to_be_sent)
  {
    send_ack();
  }

  /* we monitor the ADC and if something is wrong, a notification needs to be sent */
  if(adc_value_out_of_range)
  {
    send_alarm_packet();
    wait_until_ack_received();
  }

  /* also if the user presses a certain key, another notification is sent */
  if(user_pressed_key)
  {
    send_packet_with_keycode();
    wait_until_ack_received();
  }
}

This somewhat resembles variant a), but does not work for obvious reasons: because receive_packet() is blocking, the ADC is not monitored as long as no packets are received. Further, when the ADC is out of range, the notification packet is sent, and we can wait for an acknowledge, that's totally fine. However what happens if during that time another command is received?

My variant b) is then implemented as follows. The task1 does receive all packets and notifies others waiting for this data. The task2 performs the actual work.

void task1()
{
  pdata = ... /* allocate some memory buffer. */
  for(;;)
  {
    /* receive_packet blocks until one packet is received. pdata then contains the data. */
    receive_packet(&pdata);

    /* do somethin with pdata, e.g. process commands and so on. other tasks are
       notified with messages or event flags. */

    if(received_ack)
    {
      /* we received an acknowledge packet and can notify a waiting task. */
      ...
    }

    /* for some packets, we need to send an acknowledge. */
    if(ack_needs_to_be_sent)
    {
      send_ack();
    }
  }
}

void task2()
{
  for(;;)
  {
    if(adc_value_out_of_range)
    {
      send_alarm_packet();

      /* this could be a blocking call which only returns if task1 receives an acknowledge */
      wait_until_ack_received();
    }

    if(user_pressed_key)
    {
      send_packet_with_keycode();
      wait_until_ack_received();
    }
  }
}

while this works fine, I think it is somewhat ugly and has large overhead. So I wonder what a cleaner way of implementing this is.

\$\endgroup\$
18
  • \$\begingroup\$ Is there also a "front panel" that includes keys, buttons, etc? Or is there only a communications interface? Also, do you have any analog inputs and analog outputs? The fact that you are using a PID algorithm calls forth something else that I'd like to say. But I'm holding short until I hear the rest. Embedded software design for scientific and commercial instrumentation has been my life. So I've a few thoughts right now. But I don't know if they apply well. \$\endgroup\$
    – jonk
    May 23 '21 at 20:55
  • \$\begingroup\$ sure there is a front panel. I can perhaps guess what you want to say about the PID; however, my real problem is not the PID itself or the front panel or how I should design the LCD driver. My main problem is how I decide how to divide the work into meaningful RTOS tasks. \$\endgroup\$
    – T. Pluess
    May 23 '21 at 21:07
  • \$\begingroup\$ What's your guess about what I might say about the PID? And do you have analog inputs and outputs? And does the front panel have keys? All details matter. The entire context goes into developing a final approach that can be defended well. \$\endgroup\$
    – jonk
    May 23 '21 at 21:11
  • \$\begingroup\$ There is no one true way how to do it. Have you read the FreeRTOS book for ideas how to approach your problem? \$\endgroup\$
    – Justme
    May 23 '21 at 21:12
  • \$\begingroup\$ @Justme There are different ways to approach any given problem. Which, depends upon the more important goals of the tool. But given specific details about a situation my experience says there are only a very few remaining ways that can be defended well. Many instruments are produced that are poor because of bad design practices performed by designers who were essentially blindly groping around and barely managed to get something kind-of-working in the end. I have real-world cases to haul out if sad stories are ever needed. I care about customers so it hurts seeing bad stuff made. \$\endgroup\$
    – jonk
    May 23 '21 at 21:17
1
\$\begingroup\$

Don't do asynchronous communication unless you really, really must. You only have one communications partner. Implement a state-machine on byte-granularity. The huge advantage of that approach is that you can easily prove its correctness.

You don't even need an OS for this problem. The minimal approach uses a timer interrupt function for the PID controller.

The less code you have, the better you understand what it does, the better its quality. Don't trust code you haven't reviewed yourself at least on C level.

\$\endgroup\$
6
  • \$\begingroup\$ I know I don't need an OS for this problem, but at first glance, it seemed like it made programming the whole thing a bit easier because I not only have the control task but I also need to run a proper communication protocol which can be sort of asynchronous (e.g. a command is sent and then it takes more or less time until the acknowledge comes back etc.) \$\endgroup\$
    – T. Pluess
    May 23 '21 at 21:08
  • \$\begingroup\$ I forgot to mention that, in the end, my communication will always be sort-of asynchronous because I use interrupts to receive my data. All the interrupt does is to copy the received data to some memory location, and then, a state machine detects the individual data frames. \$\endgroup\$
    – T. Pluess
    May 23 '21 at 21:22
  • \$\begingroup\$ I advise against that idea, as you cannot handle timeout and error conditions properly that way. Or it gets really complicated. The easier way is to implement the state machine in the receiving function and only ever store valid messages in the input buffer. \$\endgroup\$
    – Janka
    May 23 '21 at 21:31
  • \$\begingroup\$ In the receiving function I store all data and then the state machine pulls the data bytes out and decides whether they are valid or not. I want to keep any ISRs as short as possible. \$\endgroup\$
    – T. Pluess
    May 23 '21 at 22:01
  • \$\begingroup\$ Um, the ISR is still super short. It's a state machine, so it stores the current state in a variable and takes this state and the received byte to decide what to do next. It doesn't do more. Just one simple action per state. \$\endgroup\$
    – Janka
    May 23 '21 at 22:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.