Skip to content

feat: added X-Crafts ERJ v2 support via the default fmc script - #3294

Open
Brutarul wants to merge 3 commits into
MobiFlight:mainfrom
Brutarul:main
Open

feat: added X-Crafts ERJ v2 support via the default fmc script#3294
Brutarul wants to merge 3 commits into
MobiFlight:mainfrom
Brutarul:main

Conversation

@Brutarul

@Brutarul Brutarul commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Summary

With this Pull Request, I have modified the default script to look for datarefs dynamically like the other Scripts do and allow the Embraer ERJ v2 Family Jets to work with all WCTRL CDUs. The problem was that X-Crafts had one Dataref that was not allowing this script to work.

An entry to the ScriptMappings.json list of airplanes and scripts has been made, so that Mobiflight can connect the WW CDU with the script.

The script was also manually tested with the listed default aircraft inside of the .json file. All tests were succesful and the script didn't break compatibility with any of the older aircraft that it was meant to support.

Acceptance criteria

  • X-Crafts ERJ v2 FMS information is displayed correctly
  • Cpt FMS working
  • FO FMS working

DoD Checklist

  • Unit tests available
    • .NET backend
  • documentation
    • User docs (docs.mobiflight.com)

Related issues

fixes #ISSUE_NUMBER

Out-of-scope

Notes

@Brutarul
Brutarul requested a review from DocMoebiuz as a code owner August 18, 2026 02:42
@github-actions

Copy link
Copy Markdown

Build for this pull request:
MobiFlightConnector.zip

@github-actions

Copy link
Copy Markdown

Build for this pull request:
MobiFlightConnector.zip

@DocMoebiuz

Copy link
Copy Markdown
Collaborator

@Brutarul please provide PR summary and other information, I provided a suggestion for "Acceptance criteria"

@Koseng

Koseng commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

@maboehme can you also have a look?

@Koseng Koseng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!
Here the review by claude. Please have a look at the suggestions and either follow a suggestion or comment why you will not.

Review Claude

Thanks for this — the direction is right. Pulling the per-line rendering out into render_fms_line() and tolerating datarefs that never arrive is the correct fix for the underlying problem, and the risk for the three aircraft already mapped to this script is low: they must currently deliver all 28 datarefs, otherwise the existing code would already die on a KeyError. So they keep the identity mapping and their rendering is unchanged apart from the (harmless) ljust.

A few things I'd like to see addressed or clarified before merge.

Important

1. The match pattern only covers the Legacy 650, not "the ERJ v2 family"

"AircraftMatchPattern": "x-crafts legacy 650" matches the Legacy 650 only. The PR title and summary promise support for "the Embraer ERJ v2 Family Jets", but an acf_ui_name such as X-Crafts ERJ 145LR or ERJ 135 will not match this pattern.

Which sim/aircraft/view/acf_ui_name strings did you actually test against? If the 135/140/145 are meant to be covered too, this needs either additional entries or a combined pattern such as x-crafts (erj|legacy).

(For what it's worth, the lowercase pattern itself is correct — ScriptRunner lowercases the aircraft name before the case-sensitive Regex.IsMatch, so it follows the existing convention.)

2. Line-count discovery mixes text and style datarefs, which can make the whole mechanism a no-op

LINE_PATTERN = r"_line(\d+)$" matches both ..._text_lineN and ..._style_lineN, and build_line_row_map() takes max() over the union of both families. That means the exact situation described in the PR summary ("X-Crafts had one Dataref that was not allowing this script to work") flips the result: if, say, text_line13 is absent while style_line13 is still listed, line_count comes out as 14, you get the identity map, and the scratchpad remapping never fires. In that case the ERJ would be working purely because of the new if text_key not in values guard, and build_line_row_map() would effectively be dead code plus an untested heuristic.

This matters because sim/cockpit2/radios/indicators/fms_cduN_* are sim datarefs that are normally present in the dataref table regardless of the loaded aircraft. Could you check what /api/v2/datarefs actually returns for fms_cdu1_ with the ERJ loaded? If all 14 lines show up there, the dynamic discovery isn't doing anything.

A more robust version would only count lines where both datarefs exist, and require them to be contiguous from 0:

text_lines, style_lines = set(), set()
for name in dataref_map.values():
    match = LINE_PATTERN.search(name)
    if not match:
        continue
    (text_lines if "_text_line" in name else style_lines).add(int(match.group(1)))

complete = text_lines & style_lines
line_count = 0
while line_count in complete:      # contiguous from 0 only
    line_count += 1

3. Failure modes are now completely silent

Previously a missing dataref raised a KeyError, the task died, and a traceback showed up in the MobiFlight log. Now build_line_row_map() returns {} (or a partial map), generate_display_json() produces 336 empty cells, and the script happily keeps sending them — the user gets a black CDU screen with not a single log entry to go on. For a script that is launched automatically based on an aircraft pattern, that is considerably harder to support.

Suggestion: log the discovered line count once (logging.info("Device %s: %d FMS lines discovered", device, line_count)), a logging.warning when the map comes back empty, and optionally a one-shot warning when a line never receives values. Important: not per frame — render_fms_line() runs at up to 10 Hz across 14 rows and would flood the log.

Medium

4. The scratchpad heuristic now applies globally, not just to the ERJ

"Last published line goes to row 13" is applied to every aircraft publishing fewer than 14 lines, including future ones. An aircraft with, say, 12 lines whose last line is not a scratchpad will silently render in the wrong row — where previously it would have failed loudly. The blast radius is small today (four patterns map to this script), but it would be good to mark this in the docstring as an assumption rather than a general rule.

Related: on the ERJ, rows 8-12 stay blank, so you get content at the top, a large gap, and the scratchpad at the very bottom. Can you confirm that's what it should look like on the device?

5. if len(style) < CDU_COLUMNS: return drops the entire line

text is padded with ljust, but a short style buffer throws away the whole row. Rendering up to min(len(style), CDU_COLUMNS) would be more consistent — you'd lose only the tail of the line instead of all of it.

6. fetch_dataref_mapping() moved into main()

The blocking urllib call now runs sequentially per device before any task starts (up to 5 s timeout each if X-Plane is unresponsive), rather than inside the task. Failure behaviour is unchanged (the script exits either way), but since this is being restructured anyway, await asyncio.to_thread(fetch_dataref_mapping, device) would be the clean version.

Summary

Good direction and low risk for the existing aircraft. Before merge I'd want points 1 (ERJ variants vs. Legacy 650 only) and 2 (text/style mixing — is the ERJ working because of build_line_row_map() or only because of the guard?) resolved. Point 3 is cheap to add and will save support effort later.

@maboehme

Copy link
Copy Markdown
Contributor

@maboehme can you also have a look?

Sorry for the slow response. I plan to take a look sometime this week!

@maboehme

Copy link
Copy Markdown
Contributor

So actually, this was less involved than I thought, and I've already taken a look.

Apart from some of the things that Claude has already commented on, this looks good to me. Thanks for further expanding the WINCTRL CDU support!

Here is my take on the points that Claude raises:

  1. The match pattern only covers the Legacy 650, not "the ERJ v2 family"

It does look to me as if the mapping for the ERJs is missing, even though they are explicitly mentioned in the script?

  1. Line-count discovery mixes text and style datarefs, which can make the whole mechanism a no-op

I think the issue that Claude raises here doesn't come up in practice -- and on top of that, I don't completely understand what Claude is trying to get at.

Claude says "This matters because sim/cockpit2/radios/indicators/fms_cduN_* are sim datarefs that are normally present in the dataref table regardless of the loaded aircraft.". If this is truly the case (have you tested?), then this would presumably apply to both the style and text datarefs, and the auto-discovery of the number of lines would have no chance of working. But as you have surely tested the code and found that it works, it appears that this concern is unfounded.

In any case, I don't see how Claude's suggestion of looking at line and style datarefs separately would help. I see how this is a defensive approach, but I'm not sure it's ever needed.

  1. Failure modes are now completely silent

Claude's suggestion of adding one-time logging for the detected number of lines makes sense to me.

  1. The scratchpad heuristic now applies globally, not just to the ERJ

True, but I don't see how this is a concern. It's not a problem for any of the aircraft supported so far. If it ever becomes an issue for a new aircraft, we'll handle it at that point.

  1. if len(style) < CDU_COLUMNS: return drops the entire line

This is something I had wondered about as well. Again, it's probably not going to occur in pratice though (at least for the aircraft supported so far), and If it does pose an issue with a future aircraft, we can again handle it at that point.

  1. fetch_dataref_mapping() moved into main()

I have to admit I'm only vaguely familiar with how Python async works and how the scripts are integrated into MobiFlight. I'm pretty sure though that if the script blocks, it doesn't block MobiFlight's UI -- and that's the only concern that I can see here. (Right?) So I don't think there's an actual issue here.

@Brutarul

Copy link
Copy Markdown
Contributor Author
  1. The Legacy is the name of all ERJ aircraft.
  2. the build line row map is needed, because the ERJ CDU works just a tiny different from the base CDUs with it's Scratchpad, that's why the previous default script did not render anything on the screen.
  3. I will add Logging
  4. I tested the default aircraft that come with xplane and are listed in the .json file. I managed to get a Display, so this script is not breaking any previous compatibility.
  5. I am not sure what you mean by this.

@github-actions

Copy link
Copy Markdown

Build for this pull request:
MobiFlightConnector.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants