C question

[kodsurf]

@RobertK66 Robert could you please explain how is it possible to :

We have rx buffer
uint8_t rx_buffer[108]

and structure that should represent some PSU /Thruster reguster / magnerometer data

typedef struct {
uint16_t val1
uint8_t val2
char  string[30];
double real_value;
.....
....
...
uint16_t some other value

}
data_structure

How is it possible with pointers to copy raw bytes from rx_buffer into structure of another type ? what is the correct syntaxis ?

should memcpy be used for that ? what is the best way to achieve it ?

Note that it is assumed that size of RX buffer fills datastructure completely

Note2 : Reason why I ask is that I believe that my thruster register parser function in current implementation is causing hard fault sometimes. Therefore I want to write it and optimize the way thruster register values would be stored

[RobertK66]

Ok, as long as I see this there is no need to copy the bytes (only if you need and reuse the rx_buffer for the next reception and want to keep the struct data independent of it. But this is another question).

In C you are empowered to use pointers as you like to interpret known memory areas.

So in this case you can do something like this

{
  uint8_t rx_buffer[108];            // makes the variable (on stack) and reserves locally 108 bytes for it
  receiveContent(rx_buffer, 108);    // somehow, somebody gives you the bytes by writing them into your variable on your stack here!)

...

  // To 'cast' this to a special struct, the only thing you have to do is make a pointer with correct type and assign it to the base adress of your memory.
  data_structure *myDsPointer;   // declares a (local) pointer (on stack) pointing to a 'data_structure' srtruct.
                                 // at this point the pointers value is not initialized so you have no idea/guarantee where it points to here. (Using it without initializing is a potential 'runaway pointer' which causes harm when data is written to its random pointed memory !) 

  myDsPointer = (data_structure *) (&(rx_buffer[0]));    // sets the pointer to point to the address ('&' !) of the first byte of the rx_buffer variable.

  myDsPointer = (data_structure *) rx_buffer;   // should have exactly the same meaning because rx_buffer (without [x]) holds already the pointer to the base address of the array it points to)

  // now you can use all values of your struct by ptr-> syntax
  uint8_t myval = myDsPointer->val2;     // should work and gives you a copy of val2 into myval variable

  ...
}

The (data_structure *) cast in front of the uint8_t pointer is only used to get rid the compiler warnings which indicate to you that you assign the values of 'incompatible pointer types'.

In the example I use only stacked local variables but if buffer and or the pointers used are 'module static' variable, the systax is exactly the same. Only difference would be that module variables are always initialized to 0x00 s after reset. Local Stack variables - i think - are never initialized and have a random content as long as you do not assign any value....

[RobertK66]

If you have to copy the data (because of your design needs rx_buffer and 'datastruct' having same content and beeing independend variables the memcpy() is the correct method to do this. To get the correct len to use with memcopy you can either use your knowledge of the magic number 108 here or you can use compiler magic with sizeof(data_structure);

another thing to be aware of is usage of compiler directives for packing structure. Use

typedef struct {
	uint8_t	val1;
	uint16_t	val2;
	uint8_t	val3;
        .....
} __attribute__((packed)) my_data_struct_name

If no packed attribute is used the compiler assumes that all values are 32bit alligned and will introduce the needed bytes to fill up or overread the 8/16 bit values within the struct ....
So checking what the compiler thinks how big your struct is (by using sizeof()) is a good practice to be sure the memory looks like you want it to be. Also examine the struct variable in Debugger can help to get the right picture here !

[RobertK66]

@Moza3105 : I just remembered here that you are familiar with C- Syntax. Maybe you could have answered this q also ;-). But never mind. Let's see how discussions advance here :-D

[kodsurf]

Thank you @RobertK66

memcpy would be appropriate in case of thruster.

rx_buffer gets overwritten always. It can receive messages of OK type which does not carry any useful register values... Buffer has to be recorded into structure only after validation.

[kodsurf]

Note that I am lucky that thruster datavector uses little endian for encoding

Otherwise it would not have worked straight away without preprocessing

[RobertK66]

good point that ;-) . Sometimes murphy works reversed .....

[kodsurf]

Another C question (related to hardfault issue)

Lets still keep it in this disscussion

This is a structure that defines "stage" function

This goes into the sequence

Question is :

I have defined an array of 6 char pointers ( 6 input arguments)

But the length of each char array is not defined !!!!!!!!!!! What happens on low level here ? Every thr_sequence_t element would have a different length because of that ?

Could this be a source of hardfault error and unexpected behavior ???

[kodsurf]

if if would be defined as multidimentional array

char thr_argv[6][MAX_ARGUMENT_LENGTH] - could this work ?

But then I would have problems here because this kind f assignment wont work

[kodsurf]

ANyway I thing that

typedef struct {

double argument1;
double argument2;

char argument3[100]

uint8_t argument4;
.....}

}

Would be much faster ....

Please advice before I do this big changes for the module

[kodsurf]

All those sequences are now loaded into RAM memory ?

Therefore those arguments have to be efficient ...

Please advice

[RobertK66]

ok, as I see this at this moment, the point here is not so much efficiency. It is the type of making it possible to define methods with 'generic' parameters. You copied this from debug uart commands, where the source of command is a 'typed in Line' so the argc argv thing is a natural fit there. In your case the 'source' of this calls is the 'Sequencer Script' which has to hold all the Data for the triggered method calls. Thats where I introduced my "Hmmm... I have to think about this ...." comment in the review.

I think we should schedule some meeting (think online will be ok) to discuss options here. With your analyses that the current implementations 'lacks' space for the real strings behinf the 6 pointers to string, I think you are correct that this causes deep problem here ;-). Somehow the real strings have to be placed 'behind the pointers'. Not sure how you did this in the hardcoded Sequences but it is for sure no trivial thing to make this design fly :-D as it is.....

[kodsurf]

Where hardcoded sequences are codded you can see in comments

Action 13310 - set heater mode ...

Instead of full string we can use uint32_t action id= 13310 for logging

Thus refered to empulsion sequence definition in their documents

[RobertK66]

With your current design and preparing the Sequences, I think you get 'around that problem with help of the compiler:

e.g.

THR_HARDCODED_SEQUENCES[sequence_id_int].sequences[exeFunc_index].function = GeneralSetRequest_sequence;
		THR_HARDCODED_SEQUENCES[sequence_id_int].sequences[exeFunc_index].thr_argv[0]= sequenc_id_char;
		THR_HARDCODED_SEQUENCES[sequence_id_int].sequences[exeFunc_index].thr_argv[1] = "14";
		THR_HARDCODED_SEQUENCES[sequence_id_int].sequences[exeFunc_index].thr_argv[2] = "1";
		THR_HARDCODED_SEQUENCES[sequence_id_int].sequences[exeFunc_index].thr_argv[5] = "\nAction 31008: Reservoir Heater Power Ref 6 W\n";

On the right side of the assignments you hardcode some strings. (This would be the equivalent of a typed paramter at some position of a CLI). This Constant string is placed by compiler somwhere in Flash-ROM. The left side (beeing the pointer to some char - string) is then assigned to point to this const string in flash. As long as you only read there this should not be a problem. But as this is const you can never expect to write something to this strings from code (which you would need if data comes from 'persitence mram memory' or ground station....

[RobertK66]

But I think using this struct you will never be able to put a complete script into MRAM or upload/download it from ground station because there is no way to have all possible Strings for an uint16 par (or whatever) prepared already in Flash ROM ! ;-)...
So you need to refactor the sequence script structs to have place for the parameters within the struct. And again here it starts to become 'uncomfortable' to use strings iso real variable values because of all this converting....

.. so pls. give me some time to come up with some constructive refactoring approach, and lets schedule a teams meeting ...

[kodsurf]

F.Y.I

I am able to change arguments of function in the sequence after initialization.

I tested that and it works.

My idea was that it is possible to change arguments and functions of existing sequences, and add new functions and arguments

[kodsurf]

So original idea was that all those arguments are dynamic and can be changed

[kodsurf]

Strings are just debug messages for logging ( for me to understand what is going on and on which "stage" of sequence the hard fault happens)

No need to keep argument that represent "custom print message" for flight sw.

Other arguments that should work in sequence are ints or doubles

[RobertK66]

But this THR_HARDCODED_SEQUENCES[sequence_id_int].sequences[exeFunc_index].thr_argv[1] = "14";
THR_HARDCODED_SEQUENCES[sequence_id_int].sequences[exeFunc_index].thr_argv[2] = "1";

"14" and "1" are const strings! How do you change this to be e.g. "15 and "2" !?

[kodsurf]

//*** THIS IS TEST EXAMPLE TO DEMONSTRATE THAT IT IS POSSIBLE TO CHANGE EXECUTION FUNCTION AND ARGUMENTS IN A SEQUENCE
//char temp_arg[1];
//sprintf(temp_arg, "%d",procedure_id);
//THR_HARDCODED_SEQUENCES[procedure_id].sequences[0].thr_argv[0] = temp_arg;
//THR_HARDCODED_SEQUENCES[procedure_id].sequences[0].thr_argv[1] = "20";
//THR_HARDCODED_SEQUENCES[procedure_id].sequences[0].thr_argv[2] = "3000";
//THR_HARDCODED_SEQUENCES[procedure_id].sequences[0].function = GeneralSetRequest_sequence;
//**************************************

Line 681 of layer4
thr_execute_sequence_cmd()

I tested that when "trigger sequence" cmd received. I substituted arguments and the function itself. It worked

[kodsurf]

Anyway -

I will TRY to change argument from char to structure today evening.

Will do it with one of existing sequences and see how would it work

[RobertK66]

No, you do not get the ppoint here.
Only to make another 'hardcoded string is not what I meant to use different pars. If we decide to use "2788" at ground station iso your hardcoded/test to change coded "3000" there is no way your software has this string available in Flash ROM ;-) .....

And your solution to make the array char*[6][max_parlen]; is not a very good one, because here the array ALWAYS takes the space of the maximum possible pars (and this times the maximum possible par-count..... This is all but memory efficient!

But anyway as you are trying to think it over already so lets talk somewhen the next days...

[kodsurf]

I think I see what you mean ...

With existing configuration that works I see compiler message that RAM is already used for 93%

If I try add another datastructure somewhere within sequences - project is not compilable any longer due to 110% memory usage

[kodsurf]

RAM usage significantly depend on number of hardcoded sequences (to which we allocate memory)

and maximum depth of sequence.

---

With 10 hardcoded sequences and depth of 50 I already get 103% RAM usage ( not compiled) .......

---

Different sequences have different lengths. No need to allocate memory for maximum depth. But how then ? Yeah we need a separate meeting to discuss it . Please suggest date.

[kodsurf]

Ok ok ok now I am sure I understood what you mean and I think I can propose solution

so instead of having arguments data structure for every "stage" of a sequence. We can declare it as a pointer to a datastructure.

Then when "hardcodding" sequence we do

declare argument structure with const so that it lives (hardcoded) into FLASH memory.

When I compile this - I see that RAM usage is significantly lower. All hardcoded structures will belong to FLASH memory and will not occupy more memory then needed. Because sequence would be just an array of pointers to datastructures but not structures inself

Would this approach work ?

---

What about the compiler warning ?

[kodsurf]

How I HARDCODED sequences before was to put separate "wait" stage in between two requests.

Instead of this I can add waiting into GeneralSet/ReadRequest() itself. This will reduce total number of stages in a sequence BY HALF !!!

I think it would be good approach

[kodsurf]

`//l4_stage_arguments_t HARDCODED_STAGE_ARGS[MAX_HARDCODED_SEQUENCES][MAX_EXECUTION_SEQUENCE_DEPTH];
l4_stage_arguments_t HARDCODED_STAGE_ARGS[2][10];

void initialize_hardcoded_thr_sequences(){

/// **************** PREPROGRAMM SEQUENCES HERE ****************
uint8_t exeFunc_index=0; // this is helper index to simplify HARDCODDING sequence manually.
int sequence_id_int=0;
uint32_t wait_between_stages = 2000;

// Action 31001:Set Operational Mode 0 / Register 0x0E  14
THR_HARDCODED_SEQUENCES[sequence_id_int].sequences[exeFunc_index].function = l4_GeneralSetRequest_sequence;
HARDCODED_STAGE_ARGS[sequence_id_int][exeFunc_index].sequence_id =sequence_id_int;
HARDCODED_STAGE_ARGS[sequence_id_int][exeFunc_index].register_index = THR_SPECIFIC_IMPULSE_REF_REG;
HARDCODED_STAGE_ARGS[sequence_id_int][exeFunc_index].double_arg1 = 3000;
THR_HARDCODED_SEQUENCES[sequence_id_int].sequences[exeFunc_index].stage_args = &HARDCODED_STAGE_ARGS[sequence_id_int][exeFunc_index];
exeFunc_index++;



 //Wait Between SET requests
THR_HARDCODED_SEQUENCES[sequence_id_int].sequences[exeFunc_index].function = l4_wait;
HARDCODED_STAGE_ARGS[sequence_id_int][exeFunc_index].sequence_id =sequence_id_int;
HARDCODED_STAGE_ARGS[sequence_id_int][exeFunc_index].register_index = THR_SPECIFIC_IMPULSE_REF_REG;
HARDCODED_STAGE_ARGS[sequence_id_int][exeFunc_index].double_arg1 = 3000;
HARDCODED_STAGE_ARGS[sequence_id_int][exeFunc_index].wait = wait_between_stages;
THR_HARDCODED_SEQUENCES[sequence_id_int].sequences[exeFunc_index].stage_args = &HARDCODED_STAGE_ARGS[sequence_id_int][exeFunc_index];
exeFunc_index++;


// Action 31001:Set Operational Mode 0 / Register 0x0E  14
THR_HARDCODED_SEQUENCES[sequence_id_int].sequences[exeFunc_index].function = l4_GeneralSetRequest_sequence;
HARDCODED_STAGE_ARGS[sequence_id_int][exeFunc_index].sequence_id =sequence_id_int;
HARDCODED_STAGE_ARGS[sequence_id_int][exeFunc_index].register_index = THR_SPECIFIC_IMPULSE_REF_REG;
HARDCODED_STAGE_ARGS[sequence_id_int][exeFunc_index].double_arg1 = 3000;
THR_HARDCODED_SEQUENCES[sequence_id_int].sequences[exeFunc_index].stage_args = &HARDCODED_STAGE_ARGS[sequence_id_int][exeFunc_index];
//exeFunc_index++;

THR_HARDCODED_SEQUENCES[sequence_id_int].length = exeFunc_index; // MANUALLY DEFINE LENGTH OF SEQUENCE //
THR_HARDCODED_SEQUENCES[sequence_id_int].sequence_trigger = false;
THR_HARDCODED_SEQUENCES[sequence_id_int].repeat = false;
THR_HARDCODED_SEQUENCES[sequence_id_int].substage_index = 0; //DEFAULT SUBSTAGE INDEX`

Only 58% ram memory usage this way. But due to

all arguments are still in ram.