MultiWii

When adding new features, it is often desirable to transmit debug information from the running copter firmware to the GUI. While the GUI provides 4 numeric variables, not every type of data is suitable for this kind of representation. This patch adds a define "DEBUGMSG", implementing a string (and ring) buffer that is able to hold debug information which can be collected and printed by the GUI.

Please consider merging these patches in firmware and GUI:
https://github.com/wertarbyte/multiwii- ... 08832afd6e
https://github.com/wertarbyte/multiwii- ... 376b5f78a9

Tommie wrote:When adding new features, it is often desirable to transmit debug information from the running copter firmware to the GUI. While the GUI provides 4 numeric variables, not every type of data is suitable for this kind of representation. This patch adds a define "DEBUGMSG", implementing a string (and ring) buffer that is able to hold debug information which can be collected and printed by the GUI.

Please consider merging these patches in firmware and GUI:
https://github.com/wertarbyte/multiwii- ... 52bd846167
https://github.com/wertarbyte/multiwii- ... 376b5f78a9

This is a very good idea... it's up to Alex, but I think it's worth to check into the source before 2.1...
Tommie, did you check that the void debugmsg_append_str(char *str) {};
nautralises all debug strings in the compiled code ?

EOSBandi wrote:This is a very good idea... it's up to Alex, but I think it's worth to check into the source before 2.1...
Tommie, did you check that the void debugmsg_append_str(char *str) {};
nautralises all debug strings in the compiled code ?

I added a few hundred lines of that call in the code, it did not increase binary size a single byte without enabling DEBUGMSG, so I guess it's working as planned.

must use MSP_IDENT.capability before adding new messages ( MSP_DEBUGMSG must be send only if the uav support this feature )

this is mainly a dev tools and dev are all skilled to merge this feature on demand . if this is directed to the user , a completed feature must presented.

do not merge this in 2.1 , no last minute prototype please, 2.1-r949 is out and you can not introduce new feature in it .

Why? Nothing bad happens if MSP_DEBUGMSG is sent to a vehicle not supporting the message; an error message is returned, so the client knows that the message type is unknown to the copter. The same happens if the capability is not compiled in (DEBUGMSG not defined). Merging the feature on demand sucks: It's only a matter of time until the patch gets out of date and cannot be applied anymore (see my patch regarding MSP_HEADING, whose message ID is now used by the waypoint code).

I agree with Tommie, the serial code is prepared to handle optional messages.And indeed this is a dev feature, but devs are frequently facing issues on other guys copters. Having the debug code in place which can be activated by a simple config statement is really helpful to debugging issues raised here on the board.
So I'm still supporting this change, but also understand that Multiwii 2.1 is in feature freeze mode so it's up to alex to decide....

The other possible solution to get it into the code once 2.1 is out which is imminent.

What about merging it now that 2.1 is out?

could you explain why you need a new buffer debug_buf, and why you don't use the TX buffer for this purpose ?

Because I only want to fill the transmit buffer if the GUI actually requests debug messages. Placing debug strings in the TX buffer without actually having a request for them might corrupt other GUI messages due to overflow.

Just rebased: https://github.com/wertarbyte/multiwii- ... /debug_msg
Any more feedback?

merged http://code.google.com/p/multiwii/source/detail?r=1000

we have to think about a specific appender for those messages in the gui

Marc and Tommie,

I think the discussion on this mod proposition is not over.
I'm still convinced we can share the TX buffer to transport those messages.

+ I don't see any reason for the GUI to ask for those messages

i am also no happy with the gui , but we have to start from somwhere , 2.1 is out and there is a clean tag , so we can procide any patch we want for fix an maintenance .


this is only the initial patch , it may be rework .. and it il will be improved : can you rework the tx buffer i will rework the gui side ?

But this is the way every other communication with the GUI works. The GUI requests data, the copter responds. Changing that will cause problems. The TX buffer is a ring buffer, which means that flooding it with debug data will overwrite any other information waiting for transmission.

If the copter keeps spewing debug messages at the serial interface, no other data will ever make it out, since any response to an actual request from the GUI will be drowned in debug messages.

Using a separate buffer provides a best effort method for debug strings: They will get accumulated in there until the GUI actually requests them (or until they expire and are overwritten), emptying the buffer with each iteration of the request loop. This is how every other GUI request is handled, and that way excessive debug messages will only overwrite older debug messages instead of corrupting other serial transmissions.

treym wrote:i am also no happy with the gui , but we have to start from somwhere , 2.1 is out and there is a clean tag , so we can procide any patch we want for fix an maintenance .


this is only the initial patch , it may be rework .. and it il will be improved : can you rework the tx buffer i will rework the gui side ?

as a side note , i can also do it on my own .. just tell me if this sound as a better solution to you .

does this mean from now on we are going to see the code cluttered with stuff like


Debugging via printf style had gone out of style - when - 90ies? Since arduino does not provide proper debug hardware interface, seems we have to bear with printf style. But why does that have to go to a GUI? Any serial terminal should be good enough for that? Even arduino IDE has one.
So debugging / printf can be done with existing serial functions already - no need to add anything. I have used that already in the past.

Hamburger wrote: I have used that already in the past.

dont tell , show .

unless you come with a practical solution ?

Hamburger wrote:does this mean from now on we are going to see the code cluttered with stuff like

Code: Select all

      debugmsg_append_str("initialization completed\n");

No, it means that you can place such messages into the code to debug your stuff without breaking the GUI communication.

But why does that have to go to a GUI? Any serial terminal should be good enough for that? Even arduino IDE has one.

And your Arduino IDE can also decipher the other GUI information? There are situation where you might want debug messages alongside Gyro/GPS/... data.

As I already pointed out: Nobody is taking anything away from you. If you don't want/like/use this stuff, just don't #define DEBUGMSG.

Tommie wrote:

Hamburger wrote:does this mean from now on we are going to see the code cluttered with stuff like

Code: Select all

      debugmsg_append_str("initialization completed\n");

No, it means that you can place such messages into the code to debug your stuff without breaking the GUI communication.

Yes, it means exactly that - code is getting cluttered with stuff like that. That is a quote from MultiWii.ino as of now (r1000).

If you don't want/like/use this stuff, just don't #define DEBUGMSG.

That aspect I like a lot.

Hamburger wrote:

If you don't want/like/use this stuff, just don't #define DEBUGMSG.

That aspect I like a lot.

So there's something for everyone to enjoy.

treym wrote:

Hamburger wrote: I have used that already in the past.

dont tell , show .

unless you come with a practical solution ?

What is it that drives you challenging me again?

Tommie wrote:But this is the way every other communication with the GUI works. The GUI requests data, the copter responds. Changing that will cause problems. The TX buffer is a ring buffer, which means that flooding it with debug data will overwrite any other information waiting for transmission.

A debug message is a kind of unsolicited message, so no need to ask for it and multiwii can output it continually.
The current implementation is perfectly compatible with this principle.

If the copter keeps spewing debug messages at the serial interface, no other data will ever make it out, since any response to an actual request from the GUI will be drowned in debug messages.

You rely on a local policy : no more than 16 chars is allowed to be transmitted per cycle.

Using a separate buffer provides a best effort method for debug strings: They will get accumulated in there until the GUI actually requests them (or until they expire and are overwritten), emptying the buffer with each iteration of the request loop. This is how every other GUI request is handled, and that way excessive debug messages will only overwrite older debug messages instead of corrupting other serial transmissions.

I don't think it's the proper way.
It should be up to you *at the source* to ensure you won't overflow the pace, and not via an intermediate buffer that could be overwritten.
Suggestion:
- increase a little the TX buffer size when debug msg is used
- consider the debug messages
--- are always serialized is the buffer is space free enough
--- are discarded otherwise
this current condition should probably be tuned for this purpose
if (bytesTXBuff > TX_BUFFER_SIZE - 40 ) return; // ensure there is enough free TX buffer to go further (40 bytes margin)


Marc, if you have a better implementation in mind, yes feel free to show it.

i would tend to change the serial decoder to output any unprocessed character to the consol buffer .. or simply displaying all the char buffer expect the correctly decoded messages . thats should do exactly what you want ;, displaying all the debug msg from the copter

Alexinparis wrote:A debug message is a kind of unsolicited message, so no need to ask for it and multiwii can output it continually.
The current implementation is perfectly compatible with this principle.

So if I print a debug message during startup (for whatever reason) and don't have my GUI already connected (think Bluetooth), the message is lost?

treym wrote:i would tend to change the serial decoder to output any unprocessed character to the consol buffer .. or simply displaying all the char buffer expect the correctly decoded messages . thats should do exactly what you want ;, displaying all the debug msg from the copter

not very clean also they need at least to be encapsulated with a proper MSP_ header+tail

this is a debug message while doing some dev work .. not a user freindly message to be send other a twitter account , we dont mind if the message is lost .

we should add a header for debug message (like the unknoww err cmd ! ) and a tail

and we will not add more debug message statement other than the default one , so user can add the messages they want .. without cluttering the repo code .

treym wrote:this is a debug message while doing some dev work .. not a user freindly message to be send other a twitter account , we dont mind if the message is lost .

But if I am the developer, I *do* want to see that message. It's the reason I added it there, e.g. for debugging some sensor initialization. That's what the debug_msg buffer was placed there for in the first place, it ensures that the message can actually be delivered.

we should add a header for debug message (like the unknoww err cmd ! ) and a tail

Like it already exists? Debug messages are just normal messages of the serial protocol.

and we will not add more debug message statement other than the default one , so user can add the messages they want .. without cluttering the repo code .

Sure. That one was more or less just a example that made it into the final patch.

if you are a developer you do want to connect the gui to see the message

If you are using bluetooth, you cannot connect to the rfcomm device until the bluetooth connection has been established - which takes a few seconds and can only be done after applying power to the board. So once your GUI is ready to send and receive data, it's probably already to late and the debug message has already passed.

for this case a delay loop can be introduced so the bluez stack is up befor the serial buffer get feed ..

#if defined(debugs_msg ) && defined(bluez_debuglink )
doDelayUntilTheSerialComIsUp
#endif


or wait for a swith change and you can do a led blink to let know the user

treym wrote:if you are a developer you do want to connect the gui to see the message

Can you give an example for when you'd need the GUI?
Yes, the GUI does show the live sensor data, those are getting updated constantly. Then you get a debug string and the data keep changing...
How does that help you - how do you know which sensor data belonged to that debug string?
Imho you cannot. In that case debug string in the GUI has no benefit over simple serial output.
On the other hand, if you simply print everything you need to the serial, it will be in sequence automatically; and without additional code.

So you have never used the debug[] variables to display arbitrary data in the GUI? Transporting strings from copter to GUI is just an extension of this mechanism.

I can give you an example: My data transfer framework, I am manipulating various copter systems, e.g. switching AUX channels, activating checkbox items etc.
But when doing modifications, it is often unclear whether a problem lies within the data decoder or occurs during the evaluation of the received commands. Adding debug statements helps greatly in cases like that.

Hamburger wrote:[On the other hand, if you simply print everything you need to the serial, it will be in sequence automatically; and without additional code.

i would simply ignore your comment .. you are very predictable and i tend to avoid toxic people like you.

did you notice that this point was already proposed ?

Alexinparis wrote:

treym wrote:i would tend to change the serial decoder to output any unprocessed character to the consol buffer .. or simply displaying all the char buffer expect the correctly decoded messages . thats should do exactly what you want ;, displaying all the debug msg from the copter

not very clean also they need at least to be encapsulated with a proper MSP_ header+tail

treym wrote: you are very predictable

I take that as a compliment, whatever else you wanted to express.

did you notice that this point was already proposed ?

???

treym wrote:if you are a developer you do want to connect the gui to see the message

now do you want to run the debug stuff through the gui or do you not?
Do you still want the debug messages to be polled from the GUI or not?

Guys PLEASE ! Do not start it again.
Patch was committed to _shared. I personally think this is a good debugging tool, especially when you have to check different states. I'll integrate it to the upcoming patch release of WinGUI too.

Hamburger wrote:

treym wrote: you are very predictable

I take that as a compliment, whatever else you wanted to express.

Well maybe you shouldn't.

I've watched quite a few discussions with you now and as an outsider it was quite interesting to watch.
I really think you are poisoning the well here.

Are you the head of quality assurance? Does every patch have to be approved by you?
Do you make the relevance judgements by yourself?

Because that would be quite the joke coming from a guy who peddled helicopter integration to a multirotor project and wastes time and resources on LCD configuration stuff nobody uses since BT modules cost 5 bucks.

I dunno what everybody else is thinking but I am starting to get the impression that you overplayed your hand here.

i notice this is in 2.3 now. can i ask a potentially stupid question. where do i read this value in gui once enabled ?