Repository navigation
fix(megasquirt): correct MIL channel data_type from boolean to bool - #1
Merged
Merged
Conversation
The adapter schema's data_type enum is [float, int, bool, string, enum]; "boolean" is not a member. Across all adapters the value appears 307 times as float, 12 as int, 2 as bool and once as "boolean" — this MIL channel — so it is a typo rather than an intended variant. Consumers that deserialize data_type into a closed enum reject the whole adapter on this one field, which is what happened in UltraLog: the entire MegaSquirt TunerStudio adapter silently failed to load, taking its channel normalisation with it (ClassicMiniDIY/UltraLog#81).
There was a problem hiding this comment.
Pull request overview
This PR fixes a schema-invalid data_type value in the MegaSquirt TunerStudio adapter so downstream consumers that deserialize data_type as a closed enum don’t reject the entire adapter document.
Changes:
- Updates the
MILchanneldata_typefrombooleanto the schema-validbool.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Corrects a one-character typo that was silently disabling the whole MegaSquirt adapter downstream.
The MIL channel declares
data_type: boolean, but the adapter schema's enum is["float", "int", "bool", "string", "enum"]. Across every adapter in this repo the value appears307 times as
float, 12 asint, 2 asbool, and exactly once asboolean— this line — so itis a typo, not an intended variant.
It matters more than a single field suggests: a consumer deserializing
data_typeinto a closedenum rejects the entire adapter document, not just the offending channel. That is what
happened in UltraLog —
megasquirt-tunerstudio.adapter.yamlfailed to parse on every launch, andall of its channel-name normalisation went with it. UltraLog has since been made tolerant of the
misspelling (ClassicMiniDIY/UltraLog#81), but the data should be correct at the source.
Other validator findings
While confirming this fix I ran every adapter and protocol against the schemas in
schema/. Therepo does not currently pass its own validator, and there is no CI running it, which is why
this shipped. Nothing below is changed in this PR — flagging it separately:
Adapters — 6/9 valid
megasquirt-tunerstudio—category: diagnosticsis not in the schema's category enum, andfile_format.header_rowis required but absenthaltech-nsp—header_row/data_start_rowuse-1as a sentinel, but the schema setsminimum: 0;timestamp_column/timestamp_unitare null where a string is requiredemerald-lg— channels carryinternal_id, which the schema disallowsProtocols — 4/9 valid
ecumaster-emu-broadcast—configurable_base_id,default_base_idmaxxecu-default—base_address,base_address_configurableemtron-broadcast—byte_orderat protocol levelmegasquirt-broadcast—commentrusefi-broadcast—offseton messagesMost of these look like the schema lagging the data rather than bad data —
diagnosticsis areal category (UltraLog's own type already supports it),
-1is a meaningful "auto-detect"sentinel, and the extra protocol keys look deliberate. So the fix is probably to widen the schemas
rather than strip the fields, but that is a judgement call per field and belongs in its own PR.
Worth adding
scripts/validate-all.shto CI once the schemas and data agree, so the next typo iscaught before it reaches consumers.