Fixing Code Checks and syncronizing PTC with DxCore

[MX682X]

I think you removed a bit too much. I'm not sure however if you want to remove any Dx-Related stuff from the Wire library or just the TWI1. Also some lingering changes in the PTC library propetries

[SpenceKonde]

Was trying to fix the invalid inequalities cited here: #1169

[SpenceKonde]

Namely this construction:

 if (true == (PORTMUX.CTRLB & PORTMUX_TWI0_bm)) {

This is the same as

if (1 == (PORTMUX.CTRLB & 0x10)) 

which is

if (1 == ( either 0 or 0x10)) 

Since 1 is never equal to 0 or 0x10, that test is always false.

[MX682X]

I think it should be fine to leave TWI1 out of twi_pins.c here. Maybe it would be even worth for readability to have seperate twi_pins.c for Dx and mTC?

[SpenceKonde]

Why do you keep mentioning TWI1?

I think we are talking past eachother.

I accidentally removed more than I intended (and put it back when you pointed this out), but the problem never had a thing to do with TWI1 other than that a mistake in the configuration of my text editor left the whole thing looking commented out so I thought it was not being compiled. But that's unrelated to the actual problem which I was investigating, which was caused by

if (true == (PORTMUX.CTRLB & PORTMUX_TWI0_bm)) {

which can never be true.

[MX682X]

Why do you keep mentioning TWI1?

I'm confiused. I've written my last comment here 4 days ago and haven't posted anything since. And the first time you mention the problem with the editor was 2 days ago in another issue thread...

Anyway, what should I do with this PR now?

[SpenceKonde]

Okay I am totally confused too... I don't even know what would happen if I merged this and what it would break.

I swear we were talkingh about this weeks ago. But fuck, man I swear about all kinds of shit so that don't mean shit...

I any event, due to this issue, there is now a BAN on the following constructions:

if (true == (....)) 

and

if (false == (...))

Until or unlesss I am given a very convincing explanation of how that does anything except promote buggy, confusing to read code, the following protocols regarding this construction are in effect. Seriously, I was RIPSHIT when I discovered how that anti-pattern behaved in C. Honestly precidence of the that it had made it into the repo. It caused my skin to break out in a rash, the skies rained red with the blood of the innocent, and worst of all, it made my cat hawk up a hairball in the middle of the bedroom floor. This is, imo, among the ten worst C syntaxes

Three protocols are being implemented as countermeasures:

• Protocol 21 * is under effect for any pull requests containing this abomination. No merge shall be carried out as long as it contains that construction. If author within range, protocol 451 is to be carried out.
• Protocol 86 ** is under effect during all development effort involving the evil code construction.
  • Upon discovery, current priorities are immediately put on hold to remove the bug
  • Features which I am unable to reimplement without this abomination will be removed from the core entirely.
  • Hardware flaws qualifying for protocol 86 are to be rendered unusable prior to discarding (generally, this means cutting cables)
• Protocol 451 *** involves application of several gallons of ordinary gasoline to the author of the offending code, followed by a lit match.

* decimal 21, hex 0x16 is NACK in ascii.
** 86 (from "nix") is widespread slang in some circles, unheard of in others, for eliminating something. Protocol 86, at present, applies as well to: Unreliable or high resistance Micro USB cables. ALL dupont cable that was onece ribbon cable - the conductor is too thin for the terminals, connections horribly unreliable, etc. Does not apply to the real "DuPont", whose excellent connector design is still made and sold at a kings ransom by Amphenol. The "dupont" terminals are copies of the Harwin M20 (which introduced the crap design the clones use), and had less than 1/10th the lifespan of the original. Obviously the chinese clones are usually signficantly worse than the M20). So ribbon cablwe dupont line is also protocol 86. Note that protocol 86 must be carried out immediately, even if it means interrupting other tasks, or delaying work while waiting for replacement, non garbage, parts.
*** 451 (degrees F) is the auto-ignition point of paper (see Fahrenheit 451)

[MX682X]

You know, it's kinda ironic. the reason why I did if ([const] == [value]) was because I was told at an internship a couple years ago that this reduces the chance of accidentely writing if ([value] = [const]) and thus introducing bugs. And here we are now... Lesson learned.

In any case, based on the "Files Changed" tab, there are no changes in twi_pins.c at this point, except for a new line at the end of the file for the code checking as far as I remember, so it should be fine to merge. I'll rename it though, so it's more clear that this PR changes anything but Wire.

[SpenceKonde]

Thanks!!
I can seethe argument in favor of that construction, except when the constant being compared is true or false - though I disagree with his assement of value = const instead of value == const.
The problem comes when type promotion happens in a strange and counterintuitive way.

I was so revolted by how that construction was interpreted by C (like everyone else, I assumed it worked the way any sane reader would expect without pondering C type promotion rules), and by the the severe nature of the bugs introduced, I decided something had to be done. My initial search of the codebase turned up no other blasphemies of this type, but I haven't checked the other cores. I'm only referring to if (true == ....) and (if (false == ....) because those behave in whacky ways.

[MX682X]

I haven't checked the other cores

Well, the DxCore does have it, but only because it's the same file, so it's inside an #ifedef.

A little offtopic, I would really appreciate if you could upgrade to avrdude 8.0 because with that it would be possbile to add the PICkit5, the PICkit4 in PIC mode and SNAP to the list of programmers, the first two even with support for the HV Pulse,