Re: [RFC PATCH v2 07/12] rv: Adapt the sco monitor to the new set_state

[Gabriele Monaco <gmonaco@redhat.com>]

On Mon, 2025-05-19 at 10:42 +0200, Nam Cao wrote:
> On Wed, May 14, 2025 at 10:43:09AM +0200, Gabriele Monaco wrote:
> >  	.function = {
> > -		{     thread_context_sco,
> > scheduling_context_sco,          INVALID_STATE },
> > -		{          INVALID_STATE,         
> > INVALID_STATE,     thread_context_sco },
> > +		{     thread_context_sco,          INVALID_STATE,
> > scheduling_context_sco,          INVALID_STATE },
> > +		{          INVALID_STATE,
> > scheduling_context_sco,          INVALID_STATE,    
> > thread_context_sco },
> 
> This is over the 100 column limit.
> 
> I know it is not your fault, this is generated. Back when I was
> playing
> with DA monitor, I made a patch to fix this. Maybe you could include
> it in
> your series?
> 
> From b4fb648398a29a9c0d8e08bd12394978d3948a5e Mon Sep 17 00:00:00
> 2001
> From: Nam Cao <namcao@linutronix.de>
> Date: Fri, 15 Nov 2024 14:56:33 +0100
> Subject: [PATCH] tools/rv/dot2c: Fix generated files going over 100
> column
>  limit
> 
> The dot2c.py script generates all states in a single line. This
> breaks the
> 100 column limit when the state machines are non-trivial.
> 
> Change dot2c.py to generate the states in separate lines.
> 
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
>  tools/verification/rvgen/rvgen/dot2c.py | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/verification/rvgen/rvgen/dot2c.py
> b/tools/verification/rvgen/rvgen/dot2c.py
> index 6009caf568d9..abc0ee569b34 100644
> --- a/tools/verification/rvgen/rvgen/dot2c.py
> +++ b/tools/verification/rvgen/rvgen/dot2c.py
> @@ -152,29 +152,22 @@ class Dot2c(Automata):
>          max_state_name = max(self.states, key = len).__len__()
>          return max(max_state_name, self.invalid_state_str.__len__())
>  
> -    def __get_state_string_length(self):
> -        maxlen = self.__get_max_strlen_of_states() +
> self.enum_suffix.__len__()
> -        return "%" + str(maxlen) + "s"
> -
>      def get_aut_init_function(self):
>          nr_states = self.states.__len__()
>          nr_events = self.events.__len__()
>          buff = []
>  
> -        strformat = self.__get_state_string_length()
> -
>          for x in range(nr_states):
> -            line = "\t\t{ "
> +            buff.append("\t\t{")
>              for y in range(nr_events):
>                  next_state = self.function[x][y]
>                  if next_state != self.invalid_state_str:
>                      next_state = self.function[x][y] +
> self.enum_suffix
>  
>                  if y != nr_events-1:
> -                    line = line + strformat % next_state + ", "
> +                    buff.append(''.join(("\t\t\t", next_state,
> ",")))
>                  else:
> -                    line = line + strformat % next_state + " },"
> -            buff.append(line)
> +                    buff.append(''.join(("\t\t\t", next_state,
> "\n\t\t},")))
>  
>          return self.__buff_to_string(buff)
>  

Thanks for bringing this up, I'm a bit undecided on this one..

The nice thing of the current representation is that it shows a matrix
and it's relatively easy to see what each event does.
On the other hand it's true larger models do exceed quite a bit the
size limits and considering you aren't really supposed to touch this
file directly (as the script does it for you), perhaps cleaner C code
should be the priority.

I'll play with your patch and see if it negatively affects the workflow
in any way. If not, I'd include it and adapt the monitors (perhaps only
those with long lines, not really need to change all).

Thanks,
Gabriele