Skip to content

refactor: replace fake_online_state integer with typed mcu2_online_ch…#5245

Merged
brianmay merged 3 commits into
mcu2-upgraded-carsfrom
refactor/mcu2-online-state-enum
Apr 5, 2026
Merged

refactor: replace fake_online_state integer with typed mcu2_online_ch…#5245
brianmay merged 3 commits into
mcu2-upgraded-carsfrom
refactor/mcu2-online-state-enum

Conversation

@brianmay
Copy link
Copy Markdown
Collaborator

Summary

The fake_online_state field in Vehicle.Data used integer values 0–3 as magic numbers to track whether an apparent online event for MCU2-upgraded cars was a genuine wakeup or a brief subsystem check. The meaning of each value required reading multiple comments and mentally mapping integers to states.
This PR replaces it with a self-documenting atom enum field mcu2_online_check:

Old (fake_online_state) New (mcu2_online_check) Meaning
0 :idle No MCU2 check in progress
1 :probing Stream connected, awaiting first power reading
2 :confirmed_fake Stream reported power=nil — subsystem wakeup only
3 :confirmed_real Stream reported numeric power — genuine wakeup
The comment on the struct field has also been expanded to explain the MCU2 behaviour and the role of each value.
No behaviour changes. This is a pure rename/retype refactor — all pattern matches and state transitions are functionally identical to before.

Part of a larger simplification effort

This is step 1 of a planned series of mechanical refactors to make vehicle.ex easier to understand before merging the MCU2 logic into main. Subsequent steps will:

  • Move drive/charge/update DB records out of state terms and into Data
  • Eliminate the :start re-dispatch hub in favour of a direct private function
  • Extract the driving offline sub-machine into its own named function

@brianmay
Copy link
Copy Markdown
Collaborator Author

This PR was entirely done with AI (opencode). So check it carefully.

So far, looks OK to me.

It also changed some of the log messages, which was a bit weird, but those changes good to me.

…eck atom

The integer values 0/1/2/3 were undocumented magic numbers that required
reading multiple comments to understand. Replace with a self-documenting
atom enum:

  :idle            – no MCU2 check in progress (was: 0)
  :probing         – stream connected, awaiting first power reading (was: 1)
  :confirmed_fake  – stream reported power=nil, subsystem wakeup (was: 2)
  :confirmed_real  – stream reported numeric power, genuine wakeup (was: 3)

No behaviour changes.
@brianmay brianmay force-pushed the refactor/mcu2-online-state-enum branch from cfdf652 to 9b4557f Compare March 31, 2026 07:02
Elixir's gradual type checker lost track of the %Data{} struct type in
three places after a with expression reassignment and one function head.
Add explicit %Data{} = pattern matches so the type system can verify the
struct updates at the two with sites, and annotate the streaming handler's
data parameter directly in its function head.
@brianmay
Copy link
Copy Markdown
Collaborator Author

I think failing Elixir tests is standard feature of this branch. Will work on fixing that after these expected 4 PRs.

This PR probably should be squashed and merged.

@micves
Copy link
Copy Markdown
Contributor

micves commented Mar 31, 2026

I think failing Elixir tests is standard feature of this branch.

Indeed it is :) The whole concept of connecting the Stream and messing with the states during online has made quite some negative impact on the tests :(

I'm happy that some work is planned to get this closer to main.

Much better with the enum 👍
I'm a little against the name mcu2_online_check.
The check is done for all cars, but on all other non-upgraded-to-muc2 cars it will be confirmed_real straight away (as far as I have understood from people having e.g. Model 3 on the same teslamate instance as a Model S)

If the plan is to still keep it that way, could we go with a more generic name? Maybe pre_online_check ?

@brianmay
Copy link
Copy Markdown
Collaborator Author

Happy to change it to pre_online_check.

Will wait maybe a day and ensure everyone is happy with that before making the change.

The behavior of checking whether an apparent online state is a subsystem
check or genuine wakeup may affect more vehicle types than just MCU2
upgrades. Rename the field and update comments/logs to be more general:

- mcu2_online_check → pre_online_check
- Comment now mentions 'some vehicles (especially MCU2-upgraded cars)'
- Log messages remove 'MCU2' prefix for generality

No behavior changes, pure rename refactor.
@brianmay brianmay force-pushed the refactor/mcu2-online-state-enum branch from 6134ed8 to d702ac3 Compare April 1, 2026 00:57
@brianmay
Copy link
Copy Markdown
Collaborator Author

brianmay commented Apr 1, 2026

Never mind, I just made the change.

@USAFPride
Copy link
Copy Markdown

Let me know when you have it ready to test out. My time with a MCU2 car is coming to an end as a replacement has arrived.

@brianmay brianmay merged commit ba33348 into mcu2-upgraded-cars Apr 5, 2026
5 of 6 checks passed
@brianmay brianmay deleted the refactor/mcu2-online-state-enum branch April 5, 2026 22:50
@brianmay
Copy link
Copy Markdown
Collaborator Author

brianmay commented Apr 6, 2026

Now that is is merged into the mcu2-upgraded-cars branch, which should get built using.

ghcr.io/teslamate-org/teslamate:pr-4453

In theory nothing has changed with this PR (and subsequent PRs I am working on also like #5259 ), this is just refactoring of code, which should have the same exact behavior.

@swiffer swiffer added the area:teslamate Related to TeslaMate core label Apr 6, 2026
@swiffer swiffer added this to the v3.1.0 milestone Apr 6, 2026
swiffer pushed a commit that referenced this pull request Apr 6, 2026
#5245)

* refactor: replace fake_online_state integer with typed mcu2_online_check atom

The integer values 0/1/2/3 were undocumented magic numbers that required
reading multiple comments to understand. Replace with a self-documenting
atom enum:

  :idle            – no MCU2 check in progress (was: 0)
  :probing         – stream connected, awaiting first power reading (was: 1)
  :confirmed_fake  – stream reported power=nil, subsystem wakeup (was: 2)
  :confirmed_real  – stream reported numeric power, genuine wakeup (was: 3)

No behaviour changes.
swiffer pushed a commit that referenced this pull request Apr 6, 2026
#5245)

* refactor: replace fake_online_state integer with typed mcu2_online_check atom

The integer values 0/1/2/3 were undocumented magic numbers that required
reading multiple comments to understand. Replace with a self-documenting
atom enum:

  :idle            – no MCU2 check in progress (was: 0)
  :probing         – stream connected, awaiting first power reading (was: 1)
  :confirmed_fake  – stream reported power=nil, subsystem wakeup (was: 2)
  :confirmed_real  – stream reported numeric power, genuine wakeup (was: 3)

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

Labels

area:teslamate Related to TeslaMate core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants