Author Topic: Parsing UART packets / state machines / packed structs  (Read 7901 times)

0 Members and 1 Guest are viewing this topic.

Online Siwastaja

  • Super Contributor
  • ***
  • Posts: 8106
  • Country: fi
Re: Parsing UART packets / state machines / packed structs
« Reply #50 on: December 08, 2022, 11:34:16 am »
I deeply loath 'documentation' made using Doxygen because the output just lacks the one important thing: why is the code the way it is and what are the relations of functions?

Exactly, Doxygen is cancerous. The purpose is to let the programmers be lazy, and give the impression of having documentation to the higher-ups who just look at it and say "looks nice, 'have nice documentation' checkbox marked".

Just worked with Eclipse PAHO embedded mqtt client, and the Doxygen documentation is just... none:
Code: [Select]
/** MQTT Subscribe - send an MQTT subscribe packet and wait for suback before returning.
 *  @param client - the client object to use
 *  @param topicFilter - the topic filter to subscribe to
 *  @param message - the message to send
 *  @return success code
 */
DLLExport int MQTTSubscribe(MQTTClient* client, const char* topicFilter, enum QoS, messageHandler);

This is nearly useless, literally the only useful information not conveyed already by the names is "wait for suback before returning".

For example, crucial piece of information in any C library where pointers are passed to functions which modify the internal state of the library is, who is responsible of the lifetime (memory allocation) of the object? Many other similar looking functions internally do memcpy on the pointer to some internal buffer, but this particular piece of crap just copies topicFilter pointer to be used later, so you can't generate the filter string in stack and just call the function, or everything breaks down in mysterious ways. Both are valid strategies but you have to tell the user which it is, and it would have been literally 15 seconds of typing to tell me that.

Further, you can see they copy-pasted the description from some other function, so not only the documentation is lacking, it's wrong, too. Bright side is, as this documentation is useless anyway, no one reads it so no one notices the mistake, and managers are happy about the nice-looking "API documentation".

We programmers - we just look at the code and figure out what is does, how it does it - and finally, try to guess why. And when we can't figure out why PAHO, for example, checks if return codes are < 0 (errors) and then replace them with fixed -1 to hide the original error code and just tell the application about a generic error, we come into conclusion there is no sensible reason to do so, and fix the crap, removing those extra steps and make error codes propagate again.
 
The following users thanked this post: nctnico, Nominal Animal, eutectique

Offline SiliconWizard

  • Super Contributor
  • ***
  • Posts: 14297
  • Country: fr
Re: Parsing UART packets / state machines / packed structs
« Reply #51 on: December 08, 2022, 06:32:56 pm »
People often use Doxygen or similar because they were asked to produce documentation, and documentation is notoriously hard and annoying to produce. So they're looking for the easy way out just to make their bosses happy.

And yes, a few comments where it really matters, and well written code with clear structure and decent type/variable/function naming is worth 10x times that, but since it actually requires competent people to be able to judge, it doesn't work that well in the corporate world.
 

Offline NorthGuy

  • Super Contributor
  • ***
  • Posts: 3137
  • Country: ca
Re: Parsing UART packets / state machines / packed structs
« Reply #52 on: December 08, 2022, 08:05:55 pm »
Documentation may be good or bad, but it doesn't matter how it's written. It's possible to write good documentation with Doxygen, as it is possible to do the same without Doxygen.

A software engeineer is the best person to write documentation, but most don't want to do that. Furthermore, the employers often don't want to pay them for that. Therefore documentation is usually written by technical writers who do not have a clue what they're writing about. This is not much better than phony Doxygen comments.
 

Offline ledtester

  • Super Contributor
  • ***
  • Posts: 3032
  • Country: us
Re: Parsing UART packets / state machines / packed structs
« Reply #53 on: December 09, 2022, 07:12:39 am »
On the topic of documentation, here is one of the main developers of the Haskell compiler talking about a note system that has proven to be a boon to maintaining the codebase over its lifetime (which now exceeds 20 years):

Past and Present of Haskell – Interview with Simon Peyton Jones
https://youtu.be/4RuLzL_q0zs?t=44m9s

(segment starts at 44:09)
 

Online DiTBho

  • Super Contributor
  • ***
  • Posts: 3793
  • Country: gb
Re: Parsing UART packets / state machines / packed structs
« Reply #54 on: December 09, 2022, 08:09:02 am »
People often use Doxygen or similar because they were asked to produce documentation, and documentation is notoriously hard and annoying to produce. So they're looking for the easy way out just to make their bosses happy.

"Stood" is the best tool-way to produce both code skeleton and doc by design.
Kind of compromise.
The opposite of courage is not cowardice, it is conformity. Even a dead fish can go with the flow
 

Offline abyrvalg

  • Frequent Contributor
  • **
  • Posts: 823
  • Country: es
Re: Parsing UART packets / state machines / packed structs
« Reply #55 on: December 10, 2022, 10:51:29 am »
uer166, the cmd in this protocol is that byte you’ve described as a fixed 01 actually. And the “cmd” from your description is a “param” (for cmd 01 it’s a register address), check my description here: https://github.com/etransport/ninebot-docs/wiki/protocol
This protocol is quite relaxed, lost packets are not a problem at all since the data changes slowly and is cached on both sides. The original M365 ECU maintains a full 256-byte cache of BMS’s registers and treats valid cmd 01 replies as cache syncs. There is no “request-response” pairing at all, the ECU is a real time device, it just sends read requests periodically and a separate superloop “task” handles responses in it’s time slots, caching new data at offsets specified by “param” field of the reply. All other ECU tasks use cached data.

What are you trying to achieve? Porting VESC to M365?
 
The following users thanked this post: uer166

Offline uer166Topic starter

  • Frequent Contributor
  • **
  • Posts: 872
  • Country: us
Re: Parsing UART packets / state machines / packed structs
« Reply #56 on: December 11, 2022, 04:04:36 am »
What are you trying to achieve? Porting VESC to M365?

This is a new experimental dual BLDC (so I can have 2 hub motors) of my own design. I need to know the pack status and voltages, but no attempt is made to keep any compatibility of the original M365 bits with the exception of pack. With this much feedback there's lots to think about, although TBH I already have it working well enough for my needs. No decisions (e.g. graceful torque de-rating) would be made here based on the BMS data so the stakes aren't too high.
 

Offline Perkele

  • Contributor
  • Posts: 42
  • Country: ie
Re: Parsing UART packets / state machines / packed structs
« Reply #57 on: December 11, 2022, 12:37:41 pm »
What you're describing is a fault of project lead, team lead or other person in charge to enforce rules. Most of people are slackers, they will invest minimum effort into uninteresting things. Documentation is boring.

Doxygen is only a tool, it all depends on how you use it. To be a bit controversial, I find it useful.
"Details" command is really useful, one might say the most important one, although rarely used in most of documentation.
For languages which use exceptions, a dedicated command to list and describe them is also really useful.

If making a documentation for a third party, you can make an introduction/main page with detailed desription on subpages. I find searchable and clickable HTML documents done this way more useful than a static document made in a text editor. "Just read the code, bro" might not be valid for compiled closed-source libraries. Yes, some of us still use those.

When trying to understand large codebases, graphing tool can help to figure-out the critical parts, or to compare the code with its design documentatio or class diagrams.

And at least in some editors there should be tools available to keep doxygen comments for classes and fuctions in sync and warn you if there is a mismatch. It really helps when an IDE gives has a mass rename of "refactoring" tool without a preview when executing it.
 

Offline Nominal Animal

  • Super Contributor
  • ***
  • Posts: 6171
  • Country: fi
    • My home page and email address
Re: Parsing UART packets / state machines / packed structs
« Reply #58 on: December 11, 2022, 12:47:24 pm »
Doxygen is only a tool, it all depends on how you use it. To be a bit controversial, I find it useful.
Doxygen, and things like Python docstrings (visible via pydoc3), are very useful for documenting an API, for those using the module or library.

They do not help document the intent behind the code, needed for efficiently and effectively maintaining the module or library itself.  In fact, they kinda-sorta steer away from it (documenting the programmer intent), which makes them less than useful for the code maintenance purposes.
 

Offline Sherlock Holmes

  • Frequent Contributor
  • **
  • !
  • Posts: 570
  • Country: us
Re: Parsing UART packets / state machines / packed structs
« Reply #59 on: December 11, 2022, 06:09:03 pm »
You cannot counter someone's argument by simply insinuating they are incompetent, ignoring "facts" and so on.

Unlike you, nctnico is highly competent and I deeply respect his competence. He just has a few weird "blind spots" where his opinions, which are just matter of taste which I could accept as opinions, like disliking structs in networking and preferring bit manipulation reconstruction, make him close his eyes from objective facts. I'm sure he still does excellent job.

I have offered all the relevant information, unlike you; in other words, please get out of our argument, your comments are completely unhelpful. We have enough trolls on this forum already. Thank you in advance.

I see through this Sir, when you speak it is to be regarded as "facts" as "objectively true" but the words of another (who disagrees with you) are merely "opinions" as the words of one who has "closed" their eyes, has "blind spots".

You are a brave man indeed, hiding behind a keyboard and unhesitatingly insulting me and my presumed lack of competence. Is that how you speak to coworkers who dare to disagree with you? Is that really the way you'd communicate in person? I doubt it, I bet you're as nice as pie under those circumstances, nobody who bullies as you do here, would last long in the workplace.

If you have nothing technical to add to the discussion, feel free to avoid the thread. I don't mind technical discourse but this is not useful.

I did have something to add and said that here.

My other post was not directed at you but at another who's post was - IMHO - rude, demeaning and condescending.



« Last Edit: December 11, 2022, 06:33:12 pm by Sherlock Holmes »
“When you have eliminated all which is impossible, then whatever remains, however improbable, must be the truth.” ~ Arthur Conan Doyle, The Case-Book of Sherlock Holmes
 

Offline abyrvalg

  • Frequent Contributor
  • **
  • Posts: 823
  • Country: es
Re: Parsing UART packets / state machines / packed structs
« Reply #60 on: December 11, 2022, 08:16:59 pm »
uer166, note one important thing original ECU does based on BMS data: regenerative brake current limiting.
M365 BMS (and all other Xiaomi/Ninebot BMSes) doesn't have an overcharge protection on the discharge port (a sudden cutoff during regen braking would be no fun). Instead of that the BMS reports an overvoltage status to the ECU (bit 9 of reg 0x30) and the ECU decreases the regen current gradually until the OV condition disappears.
 
The following users thanked this post: uer166


Share me

Digg  Facebook  SlashDot  Delicious  Technorati  Twitter  Google  Yahoo
Smf